Bug #28315 for Device-Cdio: Device-Cdio doesn't work with libcdio 0.78

Tue Jul 17 02:32:25 2007 martin.ferrari [...] gmail.com - Ticket created

Subject:	Device-Cdio doesn't work with libcdio 0.78
Date:	Tue, 17 Jul 2007 07:32:11 +0100
To:	bug-Device-Cdio [...] rt.cpan.org
From:	"Martín Ferrari" <martin.ferrari [...] gmail.com>

Hi, I've tried building the modules with the latest libcdio and it doesn't work correctly, because you're using incorrectly the SWIG language. From http://www.swig.org/Doc1.1/HTML/SWIG.html#n8: The C preprocessor version supports any constant integral expression as an argument to #if, but SWIG does not yet contain an expression evaluator so this is not currently supported. So, constructs like this doesn't work: #if LIBCDIO_VERSION_NUM > 76 #if DEBUGGING Also, there is a mistake with this new function (it seems that the error is in libcdio, but anyway...): iso9660_find_ifs_lsn doesn't exist, its name is iso9660_ifs_find_lsn Below is the patch I've used to build with 0.78, but is no good for older versions,. --- libdevice-cdio-perl-0.2.4.orig/device.swg +++ libdevice-cdio-perl-0.2.4/device.swg @@ -81,7 +81,6 @@ */ void cdio_destroy(CdIo_t *p_cdio); -#if LIBCDIO_VERSION_NUM > 76 /* cdio_driver_errmsg first appears in 0.77 code */ %rename cdio_driver_errmsg driver_errmsg; @@ -90,35 +89,6 @@ @return the string information about drc */ const char *cdio_driver_errmsg(driver_return_code_t drc); -#else -const char *driver_errmsg(driver_return_code_t drc); -%inline %{ -const char * -driver_errmsg(driver_return_code_t drc) -{ - switch(drc) { - case DRIVER_OP_SUCCESS: - return "driver operation was successful"; - case DRIVER_OP_ERROR: - return "driver I/O error"; - case DRIVER_OP_UNSUPPORTED: - return "driver operatation not supported"; - case DRIVER_OP_UNINIT: - return "driver not initialized"; - case DRIVER_OP_NOT_PERMITTED: - return "driver operatation not permitted"; - case DRIVER_OP_BAD_PARAMETER: - return "bad parameter passed"; - case DRIVER_OP_BAD_POINTER: - return "bad pointer to memory area"; - case DRIVER_OP_NO_DRIVER: - return "driver not available"; - default: - return "unknown or bad driver return status"; - } -} -%} -#endif /* LIBCDIO_VERSION_NUM > 76 */ /* eject_media(cdio)->return_code --- libdevice-cdio-perl-0.2.4.orig/read.swg +++ libdevice-cdio-perl-0.2.4/read.swg @@ -148,13 +148,13 @@ } p_buf = calloc(1, i_size); -#if DEBUGGING +#ifdef DEBUGGING printf("p_cdio: %x, i_size: %d, lsn: %d, blocksize %d, blocks %d\n", p_cdio, i_size, i_lsn, i_blocksize, i_blocks); #endif *drc = cdio_read_data_sectors (p_cdio, p_buf, i_lsn, i_blocksize, i_blocks); -#if DEBUGGING +#ifdef DEBUGGING printf("drc: %d\n", drc); #endif if (*drc < 0) { --- libdevice-cdio-perl-0.2.4.orig/perliso9660.swg +++ libdevice-cdio-perl-0.2.4/perliso9660.swg @@ -60,11 +60,7 @@ %constant long int RECORD = ISO_RECORD; /* When version 0.77 comes out, require it and fix this fix this. */ -#if 0 %constant long int PROTECTION = ISO_PROTECTION; -#else -%constant long int PROTECTION = 16; -#endif %constant long int DRESERVED1 = ISO_DRESERVED1; %constant long int DRESERVED2 = ISO_DRESERVED2; @@ -557,16 +553,14 @@ IsoStat_t *iso9660_find_fs_lsn(CdIo_t *p_cdio, lsn_t i_lsn); -#if LIBCDIO_VERSION_NUM > 76 /*! Given a directory pointer, find the filesystem entry that contains lsn and return information about it. Returns stat_t of entry if we found lsn, or NULL otherwise. */ -%rename iso9660_find_ifs_lsn ifs_find_lsn; -IsoStat_t *iso9660_find_ifs_lsn(const iso9660_t *p_iso, lsn_t i_lsn); -#endif +%rename iso9660_ifs_find_lsn ifs_find_lsn; +IsoStat_t *iso9660_ifs_find_lsn(const iso9660_t *p_iso, lsn_t i_lsn); /*! @@ -673,13 +667,11 @@ %newobject iso9660_dir_to_name; // free malloc'd return value char * iso9660_dir_to_name (const iso9660_dir_t *p_iso9660_dir); -#if LIBCDIO_VERSION_NUM > 76 /*! Returns a POSIX mode for a given p_iso_dirent. */ %rename iso9660_get_posix_filemode get_posix_filemode; mode_t iso9660_get_posix_filemode(const iso9660_stat_t *p_iso_dirent); -#endif /*! Return a string containing the preparer id with trailing -- Martín Ferrari

Sun Jul 22 17:10:31 2007 martin.ferrari [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #28315] AutoReply: Device-Cdio doesn't work with libcdio 0.78
Date:	Sun, 22 Jul 2007 22:10:15 +0100
To:	bug-Device-Cdio [...] rt.cpan.org
From:	"Martín Ferrari" <martin.ferrari [...] gmail.com>

I've been able to fix the problem reported. I misread swig documentation (I was reading an old manual), so I could make the conditionals work properly. I've also solved a couple of other issues I found. I'm attaching all the patches, they have a little description at the begining. Hope you find them useful. -- Martín Ferrari

Download 02_wrong_function_name.dpatch
application/octet-stream 922b

Message body not shown because it is not plain text.

Download 03_version_information_in_swig.dpatch
application/octet-stream 2.4k

Message body not shown because it is not plain text.

Download 04_wrong_handling_of_output_parameters.dpatch
application/octet-stream 4.4k

Message body not shown because it is not plain text.

Tue Jul 31 06:32:41 2007 Rocky Bernstein - Taken

Tue Jul 31 07:14:43 2007 Rocky Bernstein - Correspondence added

On Sun Jul 22 17:10:31 2007, martin.ferrari@gmail.com wrote: Show quoted text

> I've been able to fix the problem reported. I misread swig > documentation (I was reading an old manual), so I could make the > conditionals work properly. I've also solved a couple of other issues > I found. I'm attaching all the patches, they have a little description > at the begining. Hope you find them useful. > > >

Sorry I didn't reply sooner. Thanks for the patches. I've applied the BUILD.PL patch on the theory that it didn't break things for me. As for the other patches, what's in CVS is without a doubt very screwy if not wrong. But I'll have to look your changes later. As for the bugs in libcdio 0.78, the errno assignment were added to libcdio's CVS last March.

Tue Jul 31 07:14:45 2007 The RT System itself - Status changed from 'new' to 'open'

Tue Jul 31 12:17:24 2007 martin.ferrari [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #28315] Device-Cdio doesn't work with libcdio 0.78
Date:	Tue, 31 Jul 2007 17:16:57 +0100
To:	bug-Device-Cdio [...] rt.cpan.org
From:	"Martín Ferrari" <martin.ferrari [...] gmail.com>

Hi Rocky, On 7/31/07, Rocky Bernstein via RT <bug-Device-Cdio@rt.cpan.org> wrote: Show quoted text

> Sorry I didn't reply sooner. Thanks for the patches. I've applied the > BUILD.PL patch on the theory that it didn't break things for me. As for

Great! Show quoted text

> the other patches, what's in CVS is without a doubt very screwy if not > wrong. But I'll have to look your changes later.

Sorry, I don't understand... what's wrong, the code in CVS or how my patches apply? In any case, I was working with the release, not with the CVS, so I haven't tested that. Show quoted text

> As for the bugs in libcdio 0.78, the errno assignment were added to > libcdio's CVS last March.

Ah, same thing here. BTW, yesterday I found an error in memory handling in the name_translate functions in the perl package, that leads to a buffer overflow in some cases (when the translated name is as long as the original), the patch is very simple: --- libdevice-cdio-perl~/perliso9660.swg 2007-07-30 18:38:48.000000000 +0100 +++ libdevice-cdio-perl/perliso9660.swg 2007-07-30 18:39:18.000000000 +0100 @@ -445,7 +445,7 @@ %inline %{ char * name_translate(const char *psz_oldname) { - char *psz_newname=calloc(sizeof(char), strlen(psz_oldname)); + char *psz_newname=calloc(sizeof(char), strlen(psz_oldname)+1); iso9660_name_translate(psz_oldname, psz_newname); return psz_newname; } @@ -470,7 +470,7 @@ %inline %{ char * name_translate_ext(const char *psz_oldname, uint8_t i_joliet_level) { - char *psz_newname=calloc(sizeof(char), strlen(psz_oldname)); + char *psz_newname=calloc(sizeof(char), strlen(psz_oldname)+1); iso9660_name_translate_ext(psz_oldname, psz_newname, i_joliet_level); return psz_newname; } -- Martín Ferrari

Tue Jul 31 23:23:09 2007 Rocky Bernstein - Correspondence added

On Tue Jul 31 12:17:24 2007, martin.ferrari@gmail.com wrote: Show quoted text

> Hi Rocky, > > On 7/31/07, Rocky Bernstein via RT <bug-Device-Cdio@rt.cpan.org> wrote: >

> > Sorry I didn't reply sooner. Thanks for the patches. I've applied the > > BUILD.PL patch on the theory that it didn't break things for me. As for

> > Great! >

> > the other patches, what's in CVS is without a doubt very screwy if not > > wrong. But I'll have to look your changes later.

> > Sorry, I don't understand... what's wrong, the code in CVS or how my > patches apply? In any case, I was working with the release, not with > the CVS, so I haven't tested that.

I've tried 04_wrong_handling_of_output_parameters.dpatch on two computers and get the same failure in running "make test". I'll (try to) attach part of the log. It's possible or and even likely that the error is in fact in the regression test rather than the patch. But I'll have to look at later when I have more time available. Show quoted text

> > As for the bugs in libcdio 0.78, the errno assignment were added to > > libcdio's CVS last March.

> > Ah, same thing here. > > BTW, yesterday I found an error in memory handling in the > name_translate functions in the perl package, that leads to a buffer > overflow in some cases (when the translated name is as long as the > original), the patch is very simple: > > --- libdevice-cdio-perl~/perliso9660.swg 2007-07-30 > 18:38:48.000000000 +0100 > +++ libdevice-cdio-perl/perliso9660.swg 2007-07-30 18:39:18.000000000

+0100 Show quoted text

> @@ -445,7 +445,7 @@ > %inline %{ > char * > name_translate(const char *psz_oldname) { > - char *psz_newname=calloc(sizeof(char), strlen(psz_oldname)); > + char *psz_newname=calloc(sizeof(char), strlen(psz_oldname)+1); > iso9660_name_translate(psz_oldname, psz_newname); > return psz_newname; > } > @@ -470,7 +470,7 @@ > %inline %{ > char * > name_translate_ext(const char *psz_oldname, uint8_t i_joliet_level) { > - char *psz_newname=calloc(sizeof(char), strlen(psz_oldname)); > + char *psz_newname=calloc(sizeof(char), strlen(psz_oldname)+1); > iso9660_name_translate_ext(psz_oldname, psz_newname, i_joliet_level); > return psz_newname; > } > >

Thanks, again! I've applied this patch and have created a THANKS file. (You also have the distinction of being the first one to post anything here.)

$ make test /usr/bin/perl Build --makefile_env_macros 1 test t/00.load............ok 1/1# Testing Device::Cdio 0.2.4 t/00.load............ok t/01.pod-coverage....skipped all skipped: Test::Pod::Coverage 1.04 required for testing POD coverage t/01.pod.............skipped all skipped: Test::Pod 1.14 required for testing POD t/02.default.........ok t/05.ops.............ok t/06.bincue..........ok t/06.image...........ok t/06.toc.............ok t/07.iso.............ok t/07.iso1............NOK 5 # Failed test 'ISO 9660 file stats' # in t/07.iso1.t at line 49. # Structures begin differing at: # $got->[0]{LSN} = '/' # $expected->[0]{LSN} = '23' # Looks like you failed 1 test of 5. t/07.iso1............dubious Test returned status 1 (wstat 256, 0x100) DIED. FAILED test 5 Failed 1/5 tests, 80.00% okay t/07.iso2............NOK 2 # Failed test 'CD 9660 file stats: find_lsn(26)' # in t/07.iso2.t at line 39. # Structures begin differing at: # $got->{sec_size} = '26' # $expected->{sec_size} = '9' t/07.iso2............NOK 3 # Failed test 'CD 9660 file stats: stat('COPYING)'' # in t/07.iso2.t at line 43. # Structures begin differing at: # $got->{LSN} = '/COPYING' # $expected->{LSN} = '26' t/07.iso2............NOK 4 # Failed test 'Read directory: readdir('/')' # in t/07.iso2.t at line 56. # Structures begin differing at: # $got->[0]{LSN} = '/' # $expected->[0]{LSN} = '23' Argument "COPYING" isn't numeric in division (/) at t/07.iso2.t line 60. # Looks like you failed 3 tests of 5. t/07.iso2............dubious Test returned status 3 (wstat 768, 0x300) DIED. FAILED tests 2-4 Failed 3/5 tests, 40.00% okay t/07.iso3............NOK 2 # Failed test 'CD 9660 file stats' # in t/07.iso3.t at line 39. # Structures begin differing at: # $got->{LSN} = 'copying' # $expected->{LSN} = '24' Argument "COPYING.;1" isn't numeric in division (/) at t/07.iso3.t line 43. # Looks like you failed 1 test of 3. t/07.iso3............dubious Test returned status 1 (wstat 256, 0x100) DIED. FAILED test 2 Failed 1/3 tests, 66.67% okay t/10.cdda............ok t/10.read............ok Failed Test Stat Wstat Total Fail Failed List of Failed ------------------------------------------------------------------------------- t/07.iso1.t 1 256 5 1 20.00% 5 t/07.iso2.t 3 768 5 3 60.00% 2-4 t/07.iso3.t 1 256 3 1 33.33% 2 2 tests skipped. Failed 3/14 test scripts, 78.57% okay. 5/85 subtests failed, 94.12% okay.

Tue Jul 31 23:58:10 2007 martin.ferrari [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #28315] Device-Cdio doesn't work with libcdio 0.78
Date:	Wed, 1 Aug 2007 04:57:31 +0100
To:	bug-Device-Cdio [...] rt.cpan.org
From:	"Martín Ferrari" <martin.ferrari [...] gmail.com>

Hi again, On 8/1/07, Rocky Bernstein via RT <bug-Device-Cdio@rt.cpan.org> wrote: Show quoted text

> I've tried 04_wrong_handling_of_output_parameters.dpatch on two > computers and get the same failure in running "make test". I'll (try to) > attach part of the log. It's possible or and even likely that the error > is in fact in the regression test rather than the patch. But I'll have > to look at later when I have more time available.

The problems you're seeing now were what made me write that patch in the first place: the perl glue was returning an extra value, so the hash was wrongly built. Are you sure that the patch was correctly applied both to the .swg and the .pm files? If that's correct, please describe me your configuration so I can try to debug the problem. Show quoted text

> Thanks, again! I've applied this patch and have created a THANKS file. > (You also have the distinction of being the first one to post anything > here.)

Great! :) -- Martín Ferrari

Wed Aug 01 22:26:42 2007 Rocky Bernstein - Correspondence added

On Tue Jul 31 23:58:10 2007, martin.ferrari@gmail.com wrote: Show quoted text

> Hi again, > > On 8/1/07, Rocky Bernstein via RT <bug-Device-Cdio@rt.cpan.org> wrote: >

> > I've tried 04_wrong_handling_of_output_parameters.dpatch on two > > computers and get the same failure in running "make test". I'll (try to) > > attach part of the log. It's possible or and even likely that the error > > is in fact in the regression test rather than the patch. But I'll have > > to look at later when I have more time available.

> > The problems you're seeing now were what made me write that patch in > the first place: the perl glue was returning an extra value, so the > hash was wrongly built. > Are you sure that the patch was correctly applied both to the .swg and > the .pm files? > > If that's correct, please describe me your configuration so I can try > to debug the problem. >

> > Thanks, again! I've applied this patch and have created a THANKS file. > > (You also have the distinction of being the first one to post anything > > here.)

> > Great! :) >

I've put the current CVS and patches at http://bashdb.sf/net/Device-Cdio-0.2.5cvs.tar.gz You can check what's there if you want. The only glitch I had is that I had to install before testing because the shared libraries built by SWIG were not getting loaded in the tests. I'm not sure why not, probably not finding the local blib, but that's not relevant here. Debian uses a very old version of libcdio and I believe Nicolas Boullis will be putting a new version out sometime soon. The "stable" version of libcdio that Debian is uses I would have liked phased out 2 years ago, but such is life. I'd appreciate it if the Perl package could use libcdio 0.78.2 (with the CVS strotol patch from last March which will presumably get into the next Debian release) and not the buggy libcdio circa 0.76. It also means less hassle in code maintanance and potential problems for users. Thanks.

Wed Aug 01 22:52:33 2007 martin.ferrari [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #28315] Device-Cdio doesn't work with libcdio 0.78
Date:	Thu, 2 Aug 2007 03:52:11 +0100
To:	bug-Device-Cdio [...] rt.cpan.org
From:	"Martín Ferrari" <martin.ferrari [...] gmail.com>

Hi ROcky, On 8/2/07, Rocky Bernstein via RT <bug-Device-Cdio@rt.cpan.org> wrote: Show quoted text

> I've put the current CVS and patches at > http://bashdb.sf/net/Device-Cdio-0.2.5cvs.tar.gz > > You can check what's there if you want. The only glitch I had is that I > had to install before testing because the shared libraries built by SWIG > were not getting loaded in the tests. I'm not sure why not, probably not > finding the local blib, but that's not relevant here.

I'll give it a try. Show quoted text

> Debian uses a very old version of libcdio and I believe Nicolas Boullis > will be putting a new version out sometime soon. The "stable" version of > libcdio that Debian is uses I would have liked phased out 2 years ago, > but such is life. I'd appreciate it if the Perl package could use > libcdio 0.78.2 (with the CVS strotol patch from last March which will > presumably get into the next Debian release) and not the buggy libcdio > circa 0.76. It also means less hassle in code maintanance and potential > problems for users.

I've been talking with Nicolas, I also want to have 0.78 in debian. I have built a private package to test the perl module against it, and that's why I made most of the patches. I think he'll upload it soon. -- Martín Ferrari

Thu Aug 09 08:51:00 2007 martin.ferrari [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #28315] Device-Cdio doesn't work with libcdio 0.78
Date:	Thu, 9 Aug 2007 13:50:36 +0100
To:	bug-Device-Cdio [...] rt.cpan.org
From:	"Martín Ferrari" <martin.ferrari [...] gmail.com>

Hi again, On 8/2/07, Martín Ferrari <martin.ferrari@gmail.com> wrote: Show quoted text

> On 8/2/07, Rocky Bernstein via RT <bug-Device-Cdio@rt.cpan.org> wrote: >

> > I've put the current CVS and patches at > > http://bashdb.sf/net/Device-Cdio-0.2.5cvs.tar.gz > > > > You can check what's there if you want. The only glitch I had is that I > > had to install before testing because the shared libraries built by SWIG > > were not getting loaded in the tests. I'm not sure why not, probably not > > finding the local blib, but that's not relevant here.

I've just built it, adding my patches and it worked fine. All tests passed. On a side note, I'm having some problems with it (using the lib for other project). Maybe it's only a documentation problem and you can help me. On the perl side, (this is a bug to me), the main ISO9660/I?FS.pm don't inherit correctly (there's no new method) because you're not "use"ing the ISA libs. On the same topic, you're @EXPORTing new in some pms, which is a not nice namespace pollution. A problem that I still couldn't resolve, was to understand the difference between IFS and FS, I still don't get it... And other problem that's hitting me, is that the find_lsn routine returns only a relative filename, which is not useful at all, since I cannot know in which directory it is! -- Martín Ferrari

Sun Aug 12 09:49:00 2007 Rocky Bernstein - Correspondence added

This is straying pretty far from the what this system is about -- bug tracking. In fact with respect to the bug, I doubt there's been any progress at all. Your patches don't work for me and the changes don't completely work for you. For the other discussions I'm not sure what the right forum is, probably the libcdio devel mailing list. https://savannah.gnu.org/mail/?group=libcdio IFS and FS don't inherit because I don't think they should ;-). I've just updated FS.pm and IFS.pm to perhaps clarify better the difference between them, perhaps rereading that will help. And finally as for find_lsn not doing what you want, something should be added to the libcdio library and I've just done that as well. One can get this information in Perl I think by starting with the statbuf and walking up to root although I realize this is awkward which is why something was just added to libcdio's CVS. On Thu Aug 09 08:51:00 2007, martin.ferrari@gmail.com wrote: Show quoted text

> Hi again, > > On 8/2/07, Martín Ferrari <martin.ferrari@gmail.com> wrote:

> > On 8/2/07, Rocky Bernstein via RT <bug-Device-Cdio@rt.cpan.org> wrote: > >

> > > I've put the current CVS and patches at > > > http://bashdb.sf/net/Device-Cdio-0.2.5cvs.tar.gz > > > > > > You can check what's there if you want. The only glitch I had is

that I Show quoted text

> > > had to install before testing because the shared libraries built

by SWIG Show quoted text

> > > were not getting loaded in the tests. I'm not sure why not,

probably not Show quoted text

> > > finding the local blib, but that's not relevant here.

> > I've just built it, adding my patches and it worked fine. All tests

passed. Show quoted text

> > On a side note, I'm having some problems with it (using the lib for > other project). Maybe it's only a documentation problem and you can > help me. > > On the perl side, (this is a bug to me), the main ISO9660/I?FS.pm > don't inherit correctly (there's no new method) because you're not > "use"ing the ISA libs. On the same topic, you're @EXPORTing new in > some pms, which is a not nice namespace pollution. > > A problem that I still couldn't resolve, was to understand the > difference between IFS and FS, I still don't get it... > > And other problem that's hitting me, is that the find_lsn routine > returns only a relative filename, which is not useful at all, since I > cannot know in which directory it is! >

Sun Oct 16 21:44:45 2011 Rocky Bernstein - Correspondence added

libcdio 0.78 by now is pretty historical. Try again with 0.82 or 0.83git (or 0.83 which will soon be out) and version 0.3.0. If this is still a problem report the new results. Thanks.

Sun Oct 16 21:44:45 2011 Rocky Bernstein - Status changed from 'open' to 'rejected'