Re: [Libcdio-devel] [PATCH v2] add multi extent ISO9660 file support to libcdio

[Thomas Schmitt]

Hi,

Pete Batard wrote:
> As such, the old struct iso9660_stat_s fields remain accessible, but are now
> are guarded under a DO_NOT_WANT_COMPATIBILITY macro (that I am reusing as it
> was already present to guard other deprecated elements),

Looks like an option only for embedded usage of libcdio.
How to keep that second version of libcdio from ever escaping into the
wild as dynamic library ?

Some remarks about the changeset:
-------------------------------------------------------------------------

The new members of struct iso9660_stat_s must be moved to the end of the
struct, i.e. underneath "char filename".

As it is now, libcdio would lose ABI compatibility, because an already
compiled program would access in new libcdio  iso9660_stat_s.num_extents
when it wants to access  iso9660_stat_s.xt  in old libcdio.

-------------------------------------------------------------------------
  
I'm still unhappy with public
  #define ISO_MAX_MULTIEXTENT 8
and
  struct iso9660_stat_s {
    ...
    lsn_t extent_lsn[ISO_MAX_MULTIEXTENT];
    uint32_t extent_size[ISO_MAX_MULTIEXTENT];
  }

If this becomes API, then future enlarging of ISO_MAX_MULTIEXTENT would
break ABI compatibility, because the offset of extent_size in the struct
would change.

Roughly thought alternatives:

1) Change extent_lsn and extent_size from array to pointer and allocate
  dynamic memory for them.
  This wastes more space and complicates code, but is safe against future
  changes in struct iso9660_stat_s.

2) Combine extent_lsn and extent_size in a struct of which iso9660_stat_s
  contains an array:

    struct iso0660_extent_s {
      lsn_t    extent_lsn;
      uint32_t extent_size;
    }
    struct iso9660_stat_s {
      ...
      struct iso0660_extent_s extents[ISO_MAX_MULTIEXTENT];
    }

  This would be ABI-safe with changes of ISO_MAX_MULTIEXTENT as long
  as no further changes are made in struct iso9660_stat_s. (No member
  of struct iso9660_stat_s would change its offset if ISO_MAX_MULTIEXTENT
  changes.)

3) Combining both proposals (i.e. dynamic array of iso0660_extent_s) would
  reduce waste from two pointers to one pointer.
  It would avoid the heavy drawback of proposal 2 to effectively make
  impossible future ABI compatible extensions of iso9660_stat_s.

-------------------------------------------------------------------------


> Unfortunately, due to time constraints, as well as the relative complexity
> of having to contend with >4GB files, I do not think I will be in a position
> to add a test to validate the extraction of a multiextent file anytime soon.

I will try to create a really small multi-extent ISO for testing.
(There is no law that the extents except the last one must have ~ 4 GiB
 size. But to make extents of 10 KiB, i will need to hack libisofs.)


Have a nice day :)

Thomas