[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Libcdio-devel] Read Sub-channel changes

From: Thomas Schmitt
Subject: Re: [Libcdio-devel] Read Sub-channel changes
Date: Wed, 08 Jun 2016 22:47:25 +0200


> Again, we should not contemplate changing something that exists

I am well aware of the goal to keep API and ABI stable.
Further i do not want to break things without a good excuse prepared.

> It would be fine to create a function that runs a mmc command and then
> issues a sense command.  Since it issues the mmc command it knows what
> command it ran.

I understand you are in favor of this alternative:

> > - Move sense code evaluation and reporting to functions which know
> >   the mmc_cdb_t. This knowledge is currently quite low-level and very
> >   volatile.

So i will move the call of mmc_evaluate_last_sense() from
mmc_get_mcn_isrc_private() to mmc_read_subchannel().

This prevents my plan to let error indicating sense data act like
missing MCVAL/TCVAL bit. These bits are known only in
In mmc_read_subchannel() i can only return something like DRIVER_OP_ERROR.

Is DRIVER_OP_ERROR the appropriate indicator for invalid reply data ?

A report (deliberately provoked by wrong track number 4) now looks like this

  ++ WARN: READ_SUBCHANNEL (CDB: 42 00 40 03 00 00 04 00 04 00 )
  ++ WARN: [5 24 00] : Illegal Request, Invalid field in cdb

(Shall i downgrade it to INFO ?
 Ignorable problems will be reported by INFO.)

This is the call in mmc_read_subchannel which causes the warnings:

  if(mmc_evaluate_last_sense(p_cdio, &cdb, CDIO_MMC_EVAL_SENSE_WARN)
    i_status = DRIVER_OP_ERROR;

It should be executed in low level MMC functions immediately after

  i_status = MMC_RUN_CMD(SCSI_MMC_DATA_READ, i_timeout_ms);

This is not ideal because DRIVER_OP_ERROR could override other errors.
But if sense data was evaluated only if (i_status == DRIVER_OP_SUCCESS)
then ioctl(CDROM_SEND_PACKET) would obscure the detailed error message
by its errno == EIO.

I guess this is the appropriate companion macro for MMC_RUN_CMD

  #define MMC_EVAL_SENSE(i_status, report_mode) \
     if(mmc_evaluate_last_sense(p_cdio, &cdb, report_mode) \
       i_status = DRIVER_OP_ERROR;

which reduces the footprint of above evaluation to


(Shall i leave out the ';' in the macro so that its invocation needs one ?)


If this mechanism gets accepted then one would next have to sprinkle its
use all over those parts of lib/driver/mmc/ which call op.run_mmc_cmd().

Of course one may only change those code parts which one is able to test.

Have a nice day :)


reply via email to

[Prev in Thread] Current Thread [Next in Thread]