Re: [Libcdio-devel] [PATCH] Fix invalid/bad directory errors with openSUSE Leap 15.0 ISO

[Rocky Bernstein]

Pete you have my blessing. Thanks for handling.

On Sun, May 27, 2018 at 5:45 PM, Pete Batard <address@hidden> wrote:

> Hi,
>
> Taking in all the good advice from the earlier discussion, I am proposing
> to apply the attached set of patches to address the issue of processing
> openSUSE Leap, and other non specs-compliant ISOs, without bailing out or
> reporting warnings.
>
> This basically alters from_733() to remove all warnings and return the
> little endian value always, and use this call, instead of
> from_733_with_err(), in iso9660_fs.c.
>
> For good measure, from_723() is also altered in the same fashion, and a
> new from_723_with_err() is introduced, so we may use this in place where we
> may want to detect LE vs BE mismatch.
>
> I have tested this patch against the problematic openSUSE Leap 15.0 ISO
> and found that both isofile and isolist worked as expected. I have only
> tested on a little-endian arch however.
>
> I should be able to push these patches to mainline, provided I receive a
> green light from Rocky (and provided Thomas doesn't have objections about
> this proposal).
>
> Regards,
>
> /Pete
>
>
> On 2018.05.26 23:00, Thomas Schmitt wrote:
>
>> Hi,
>>
>> i wrote:
>>
>>   uint8_t *u;
>>>   u = (uint8_t *) p;
>>>   return (u[0] | (u[1] << 8) | (u[2] << 16) | (u[3] << 24));
>>>
>>
>> not enough "&":
>>
>>    u = (uint8_t *) &p;
>>
>> I should have compiled first before sending mail.
>>
>>
>> Have a nice day :)
>>
>> Thomas
>>
>>
>
> From a4155f014c640e6896a41205a0f997be8db33808 Mon Sep 17 00:00:00 2001
> From: Pete Batard <address@hidden>
> Date: Sun, 27 May 2018 22:27:27 +0100
> Subject: [PATCH 1/2] Set from_723 and from_733 to return the little-endian
>  value always
>
> * And silence the warning in case of little-endian and big-endian mismatch
> * Also introduce a from_723_with_err() that can be used to report mismatch
>   if needed
> ---
>  include/cdio/bytesex.h | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/include/cdio/bytesex.h b/include/cdio/bytesex.h
> index 0ec92463..c66ed809 100644
> --- a/include/cdio/bytesex.h
> +++ b/include/cdio/bytesex.h
> @@ -171,9 +171,20 @@ to_723(uint16_t i)
>  static CDIO_INLINE uint16_t
>  from_723 (uint32_t p)
>  {
> -  if (uint32_swap_le_be (p) != p)
> -    cdio_warn ("from_723: broken byte order");
> +  uint8_t *u = (uint8_t *) &p;
> +  /* Return the little-endian part always, to handle non-specs-compliant
> images */
> +  return (u[0] | (u[1] << 8));
> +}
>
> +static CDIO_INLINE uint16_t
> +from_723_with_err (uint32_t p, bool *err)
> +{
> +  if (uint32_swap_le_be (p) != p) {
> +    cdio_warn ("from_723: broken byte order");
> +    *err = true;
> +  } else {
> +    *err = false;
> +  }
>    return (0xFFFF & p);
>  }
>
> @@ -200,10 +211,9 @@ to_733(uint32_t i)
>  static CDIO_INLINE uint32_t
>  from_733 (uint64_t p)
>  {
> -  if (uint64_swap_le_be (p) != p)
> -    cdio_warn ("from_733: broken byte order");
> -
> -  return (UINT32_C(0xFFFFFFFF) & p);
> +  uint8_t *u = (uint8_t *) &p;
> +  /* Return the little-endian part always, to handle non-specs-compliant
> images */
> +  return (u[0] | (u[1] << 8) | (u[2] << 16) | (u[3] << 24));
>  }
>
>  static CDIO_INLINE uint32_t
> --
> 2.17.0.windows.1
>
>
> From 93a3fdc55c704f3f9bb20ab15ca8d94330f4ef7f Mon Sep 17 00:00:00 2001
> From: Pete Batard <address@hidden>
> Date: Sun, 27 May 2018 22:30:04 +0100
> Subject: [PATCH 2/2] Switch to using the more relaxed from_733 in
>  _iso9660_dir_to_statbuf
>
> * Done so that libcdio doesn't bail out when processing non-compliant
>   ISOs such as openSUSE Leap 15.0
> ---
>  lib/iso9660/iso9660_fs.c | 13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/lib/iso9660/iso9660_fs.c b/lib/iso9660/iso9660_fs.c
> index 85ad2736..33f47711 100644
> --- a/lib/iso9660/iso9660_fs.c
> +++ b/lib/iso9660/iso9660_fs.c
> @@ -784,7 +784,6 @@ _iso9660_dir_to_statbuf (iso9660_dir_t *p_iso9660_dir,
> bool_3way_t b_xa,
>    iso711_t i_fname;
>    unsigned int stat_len;
>    iso9660_stat_t *p_stat = NULL;
> -  bool err;
>
>    if (!dir_len) return NULL;
>
> @@ -801,16 +800,8 @@ _iso9660_dir_to_statbuf (iso9660_dir_t
> *p_iso9660_dir, bool_3way_t b_xa,
>      }
>    p_stat->type    = (p_iso9660_dir->file_flags & ISO_DIRECTORY)
>      ? _STAT_DIR : _STAT_FILE;
> -  p_stat->lsn     = from_733_with_err (p_iso9660_dir->extent, &err);
> -  if (err) {
> -    free(p_stat);
> -    return NULL;
> -  }
> -  p_stat->size    = from_733_with_err (p_iso9660_dir->size, &err);
> -  if (err) {
> -    free(p_stat);
> -    return NULL;
> -  }
> +  p_stat->lsn     = from_733 (p_iso9660_dir->extent);
> +  p_stat->size    = from_733 (p_iso9660_dir->size);
>    p_stat->secsize = _cdio_len2blocks (p_stat->size, ISO_BLOCKSIZE);
>    p_stat->rr.b3_rock = dunno; /*FIXME should do based on mask */
>    p_stat->b_xa    = false;
> --
> 2.17.0.windows.1
>
>
>