[Libcdio-devel] Re: Warning of potential regression

[Thomas Schmitt]

Hi,

> So is it advisable to add a check to run_mmc_cmd_linux?
> Perhaps that the length of i_buf is 0?

I have tested now this contraption

  if (SCSI_MMC_DATA_NONE == e_direction)
    i_buf = 0;
  x = CGC_DATA_NONE;
  cgc.data_direction = (SCSI_MMC_DATA_READ == e_direction) ? CGC_DATA_READ :
                       (SCSI_MMC_DATA_WRITE == e_direction) ? CGC_DATA_WRITE :
                       x;

on my Linux 2.6.18 with
x= CGC_DATA_NONE, CGC_DATA_READ, CGC_DATA_WRITE
It worked with all three.
But that is a matter of opaque driver entrails.

I am not sure whether there are no use cases
for the combination of READ or WRITE with length
0. It seems less risky for now to perform them
as it was done in the past.
After all we have no indication that it would
not work on real world systems.

I believe
   SCSI_MMC_DATA_NONE -> CGC_DATA_NONE
is the right thing to do in gnu_linux.c.
One should look for testers and then make
similar changes in those drivers where a NONE
direction is defined by the OS.
Without test better no change. For now it seems
not urgent.

The showstopper in my experiments was the
mapping
  SCSI_MMC_DATA_WRITE -> CGC_DATA_READ
but not the missing NONE direction.
(Maybe i should not have raised the NONE issue
 already now. But i was not aware that it is
 so harmless on Linux.)


> > How about a mode "MMC_RDWR" which opens for
> > writing and uses READ(10) or READ(12) via
> > mmc_run_cmd() for libcdio read functions ?
>
> I've added that to gnu_linux.c. It has only been minimally tested.

As the instigator i will have to do so.
First i'll have to learn git. (sigh)


> In places where common code is shared between
> libburn and libcdio, I'd like to see that be put out as a separate library
> and used by both projects.

One will have to invest some time into comparison
first. To share concepts and code snippets is
quite easy. To share reusable code needs more
spadework.


> > - libcdio learns to handle BD and to recognize
> >  emulated ISO 9660 multi session.
> Sure that sounds reasonable. I don't know that
> we have someone devoted to it.

I will try to find out where cd-info mistakes
the BD.

> >  (How is ISO 9660 multi-session support
> I don't know.

Well, the ISO multi-session issue seems to be a
bigger topic here.


> > - libcdio learns to read ACL and xattr from
> >  eventual AAIP entries in ISO 9660 images.
> Sure, that's fine. Again it is more a matter of someone to work on ;-)

Looks like you need a ISO-9660 appointee
overall.
(I think i should now sneak silently towards
 UDF ...)


> Use a cast or declare a variable of type generic_img_private_t on
> p_cdio->env->gen. I think something like:
>   generic_img_private_t *p_gen = p_cdio->env->gen.

I'll try. env is the trouble maker currently.

But is it wise to pierce the intended
encapsulation of the void pointer ?
Possibly there is a better place to be reachable
by both  run_mmc_cmd_linux()  and  CdIo_t.
Any idea ?


> > Shall one rather introduce a new
> > mmc_run_cmd_sense() which performs a command and
> > returns the sense bytes as parameter ?
> 
> That would be fine too. Seems a little cleaner, no?

It would be more work, i guess:
- new function pointer in cdio_funcs_t
- initializer for that in all drivers
- new functions in all applicable drivers
- default to old run_mmc_cmd_*() in the others

The recorded sense with getter function is
leaner because calloc() in all drivers takes
care of proper initialization: 0 bytes is what
we want.
Thus no changes needed in non-applicable drivers.

If only i knew a neat point where to hand over
data between driver and CdIo_t ...


Have a nice day :)

Thomas