[Libcdio-devel] ack! another bug fixed in mmc_read_cd....

[R. Bernstein]

Thanks for fixing. I don't think compatibilty (with broken behavior)
should necessarily be a factor here. The last release was the
"stability" release. The upcoming one, whenever that happens never was
planned or billed as one. (But we do need to remember to adjust shared
library version numbers accordingly before the release since
technically the API has changed)

Also, the various distributions tend to be change sloway and be a couple
release behind anyway.

But if someone does have a problem, it should be brought up here in
the mailing list. My rule generally has been that silence means
agreement. :-)

Thanks again.

Robert William Fuller writes:
 > Turns out mmc_read_cd() was ignoring the read_sector_type and treating 
 > everything as CDIO_MMC_READ_TYPE_ANY.  I do not know when this occurred 
 > but it seems like it happened at the introduction of the 
 > CDIO_MMC_SET_READ_TYPE macro.  Basically, the broken code looked like this:
 > 
 >    i_read_type = read_sector_type << 2;
 >    if (b_digital_audio_play) i_read_type |= 0x2;
 >    CDIO_MMC_SET_READ_TYPE(cdb.field, i_read_type);
 > 
 > where the macro looks like this:
 > 
 > #define CDIO_MMC_SET_READ_TYPE(cdb, sector_type) \
 >    cdb[1] = (sector_type << 2)
 > 
 > Because of the double shift, once in the macro, and once in 
 > mmc_read_cd(), the read_sector_type was being put into the reserved 
 > field of the cdb where it was ignored.  I fixed it with this code (and 
 > eliminated a local variable in the process:)
 > 
 >    CDIO_MMC_SET_READ_TYPE(cdb.field, read_sector_type);
 >    if (b_digital_audio_play) cdb.field[1] |= 0x2;
 > 
 > Of course the unfortunate side of this is that if somebody has broken 
 > code that depends on broken behavior, they'll now get an error instead 
 > of erroneous success.
 > 
 >