Re: [Libcdio-devel] Rock Ridge and libisofs/xorriso 'AL' extension

[Rocky Bernstein]

I have applied the patch in a git branch called TS-RockRidge-Fix. See
http://git.savannah.gnu.org/cgit/libcdio.git?h=TS-RockRidge-Fix

The patch didn't apply cleanly from an email cut and paste. Also, I made a
few insignificant or style changes, like removing #ifdef
(since that's what a git branch does).  So please see that branch; I may
have made a mistake and/or misunderstood something.

Nice would be to a have test case for this change. A test check for lack of
a memory leak would be great too. (The test
would only be run if valgrind was installed)


A comment in the patch that I don't understand is this:

+              The formula does not exactly round up, as it enlarges offset
+              even if it encounters (offset % ISO_BLOCKSIZE) == 0 .
+              Then the block would be completely 0. Unplausible.

I don't understand what "round up" means. Normally "rounding" for real
numbers means that if there is a fractional
part of a number the number is increased to the next integer value.

And what does "Unplausible" mean? That it is unlikely that the *size* of a
block is zero? But let's say "a block is completely 0"
for whatever that means. Does that then indicate that this code does the
wrong thing?



On Wed, Jul 26, 2017 at 10:34 AM, Thomas Schmitt <address@hidden> wrote:

> Hi,
>
> this is my proposal of a fix for the old bug
>   https://savannah.gnu.org/bugs/?45015
>
> The 50 KB test ISO yields now with valgrind:
>
>   ISO-9660 Information
>        2048 /a
>           0 /b.txt
>           3 /a/c.txt
>   ==31019==
>   ==31019== HEAP SUMMARY:
>   ==31019==     in use at exit: 0 bytes in 0 blocks
>   ...
>   ==31019== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 1 from 1)
>
> --------------------------------------------------------------------------
>
> This name out of kali-linux-2017.1-amd64.iso looks indeed like been read
> from Rock Ridge:
>
>      390174 /pool/main/g/gdk-pixbuf/libgdk-pixbuf2.0-0-udeb_2.36.
> 5-2_amd64.udeb
>
> with full ok from valgrind.
>
> But iso-info does not read by default Rock Ridge names via
> get_rock_ridge_filename() from kali-linux-2017.1-amd64.iso.
> It rather reads the Joliet tree. (And i wondered why there is suddenly
> no Rock Ridge in the directory records ...)
>
> One has to use
>
>   iso-info --no-joliet -f -i /dvdbuffer/kali-linux-2017.1-amd64.iso
>
> to force it into Rock Ridge reading.
>
> So it is now unclear why Pete did not get the long Joliet names from
> libcdio,
> which are quite the same as the Rock Ridge names.
>
>
> --------------------------------------------------------------------------
> Whatever. Here is the patch for testing.
> Undefine macro Fix_by_ts_B70726 in both files to get back to original
> behavior.
> --------------------------------------------------------------------------
>
> --- lib/iso9660/iso9660_fs.orig.c       2017-07-26 14:44:21.493300447 +0200
> +++ lib/iso9660/iso9660_fs.c    2017-07-26 15:20:36.601308660 +0200
> @@ -53,6 +53,8 @@
>  #include "_cdio_stdio.h"
>  #include "cdio_private.h"
>
> +#define Fix_by_ts_B70726 yes
> +
>  /** Implementation of iso9660_t type */
>  struct _iso9660_s {
>    CdioDataSource_t *stream; /**< Stream pointer */
> @@ -1385,7 +1387,26 @@ iso9660_ifs_readdir (iso9660_t *p_iso, c
>
>         if (!iso9660_get_dir_len(p_iso9660_dir))
>           {
> +
> +#ifdef Fix_by_ts_B70726
> +
> +           /*
> +              Length 0 indicates that no more directory records are in
> this
> +              block. So move on to the next one, which may end the loop.
> +
> +              The formula does not exactly round up, as it enlarges offset
> +              even if it encounters (offset % ISO_BLOCKSIZE) == 0 .
> +              Then the block would be completely 0. Unplausible. But to go
> +              on, it has to be skipped.
> +           */
> +           offset+= ISO_BLOCKSIZE - (offset % ISO_BLOCKSIZE);
> +
> +#else
> +
>             offset++;
> +
> +#endif /* Fix_by_ts_B70726 */
> +
>             continue;
>           }
>
>
> --- lib/iso9660/rock.orig.c     2017-07-26 15:28:32.601310457 +0200
> +++ lib/iso9660/rock.c  2017-07-26 15:30:31.733310907 +0200
> @@ -39,6 +39,8 @@
>  #include <cdio/bytesex.h>
>  #include "filemode.h"
>
> +#define Fix_by_ts_B70726 yes
> +
>  #define CDIO_MKDEV(ma,mi)      ((ma)<<16 | (mi))
>
>  enum iso_rock_enums iso_rock_enums;
> @@ -169,6 +171,9 @@ get_rock_ridge_filename(iso9660_dir_t *
>      while (len > 1){ /* There may be one byte for padding somewhere */
>        rr = (iso_extension_record_t *) chr;
>        sig = *chr+(*(chr+1) << 8);
> +
> +#ifndef Fix_by_ts_B70726
> +
>        switch(sig){
>        case SIG('S','P'):
>        case SIG('C','E'):
> @@ -188,6 +193,8 @@ get_rock_ridge_filename(iso9660_dir_t *
>         goto out;
>        }
>
> +#endif /* ! Fix_by_ts_B70726 */
> +
>        if (rr->len == 0) goto out; /* Something got screwed up here */
>        chr += rr->len;
>        len -= rr->len;
>
> --------------------------------------------------------------------------
>
> Have a nice day :)
>
> Thomas
>
>
>