libcdio-devel
[Top][All Lists]
Advanced

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

Re: [Libcdio-devel] Re: Warning of potential regression


From: Rocky Bernstein
Subject: Re: [Libcdio-devel] Re: Warning of potential regression
Date: Sun, 20 Dec 2009 17:49:33 -0500

With regards to how to handle run_mmc_cmd_linux, I don't have a strong
feeling one way or another. If it simplifies to leave things are they are
given that it's not broken, okay. If you want to test changing to make more
consistent with what is done elsewhere, that's okay too.

Thanks for undertaking to test the change to allow read/write access, and
for problems with cd-info and Blu-Ray disks.


On Sun, Dec 20, 2009 at 8:45 AM, Thomas Schmitt <address@hidden> wrote:

> 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
>
>
>
>


reply via email to

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