[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libcdio-devel] [RFC] New API iso9660_statv2_t as API/ABI compatible
From: |
Pete Batard |
Subject: |
Re: [Libcdio-devel] [RFC] New API iso9660_statv2_t as API/ABI compatible way to read files >= 4 GiB |
Date: |
Fri, 6 Jul 2018 21:03:50 +0100 |
User-agent: |
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.0 |
Hi Thomas,
On 2018.07.06 19:09, Thomas Schmitt wrote:
i have pushed the very large commit 87f5053 to branch ts-multiextent :
Wow! Indeed.
Just for the record, I'm seeing a couple of warnings when compiling with
MinGW-w64 on Windows, but the tests look all good.
Here they are:
-------------------------------------------------------------------------
make[3]: Entering directory '/d/libcdio/lib/iso9660'
CC iso9660.lo
CC iso9660_fs.lo
CC rock.lo
CC xa.lo
iso9660_fs.c: In function '_iso9660_statbuf_alloc_extents':
iso9660_fs.c:866:37: warning: format '%lu' expects argument of type
'long unsigned int', but argument 2 has type 'long long unsigned int'
[-Wformat=]
cdio_warn("Could not allocate %lu bytes",
~~^
%I64u
(unsigned long) num_extents *
sizeof(iso9660_extent_descr_t));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
iso9660_fs.c: In function '_iso9660_dir_to_statbuf':
iso9660_fs.c:914:23: warning: array subscript is above array bounds
[-Warray-bounds]
p_stat->magic_number[1] = 2;
~~~~~~~~~~~~~~~~~~~~^~~
CCLD libiso9660.la
make[3]: Leaving directory '/d/libcdio/lib/iso9660'
-------------------------------------------------------------------------
I see only two problems with his proposal:
* It breaks the ABI.
* It will break the ABI again if ever the maximum number of extents
ISO_MAX_MULTIEXTENT shall be incresed from 8 to a higher number.
It is unattractive to choose a large value already now, because each
extent slot costs 8 byte and iso9660_stat_t can have many instances.
(This could be fixed without a new API by cherrypicking parts of my
proposal. I will probably do, if iso9660_statv2_t gets rejected.)
And I am still going to maintain that, empirically, we are going to find
that both these issues, and especially the second one, are overblown.
My reasoning for that is that ABI breakage is something that happens all
the time, and that any sensible developer/OS should be able to handle in
their stride at this stage. Not to say that this is not an
inconvenience, but I think we should indeed qualify it for what it is:
an inconvenience rather than a problem. Also, as I stated before,
considering that we recently release libcdio 2.0, I don't personally
have much of an issue if we want to sit on ABI-breakine multiextent
changes for a year or two, to give some breathing space for developers
and OS/package maintainers who just carried out the 2.0 upgrade.
With regards to point #2, I am basing my _anticipation_ on the following:
I am the developer of a relatively successful libcdio-based application,
that has seen steady multi-million monthly downloads for the past few
years, and to which people are throwing all kind of ISOs. Yet, it has
only been until a few months ago that I came come across a report from
someone who was using a multiextent ISO, with a single extent.
Considering that (as my mailbox tends to demonstrate) users of my
application are not especially shy about letting me know when something
doesn't work as expected, it gives me some relatively sound basis to
think that we are _exceedingly_ unlikely to come across non-academical
ISO-9660 images that are going to use more than 8 extents in the future
and that, at best, all the work we (or, more accurately: Thomas) have
been doing is probably going to benefit less than a couple of people.
So, to me, it looks like a lot of effort to help a handful of people who
most likely would just have to recompile whatever app they are using to
use a static libcdio library where they just change the hardcoded limit,
according to their need, and, as such, it seems like a wasted effort...
Besides time constraints, this is also the reason why I preferred to
bail out, than commit to addressing what I consider to be unlikely
actual issues.
It turned out that avoiding thee two ABI problems implies to stirr up
a lot of code.
Pete is officially allowed to laugh now.
Seeing the extent of your changes, and the point that I am kind of
making against them, I really don't feel like laughing.
I have been in the same situation as you many times over, with a
changeset proposal that I put a lot of effort in (and that I would also
have no trouble to argue was technically brilliant), but that got shot
down by the original project for various reasons.
So, I do feel bad about currently leaning against your proposal, on
account that, considering that the idea is that libcdio users will
ultimately upgrade to using v2, that code upgrade to v2 looks to be a
lot more painful than simply being able to reuse a reworked current
struct, even if that results in ABI breakage. Sorting out a broken ABI
is a common issue for which you can find a lot of help. Sorting out the
use of a newly introduced custom struct is a different matter. So, as a
developer, my feeling is that, give the choice, I'd rather go through an
ABI breakage.
But of course, I'm a bit too close to one side of the debate to be
considered objective on the matter, so please feel free to take what I
am saying with a grain of salt. This is even more more true as, in my
application, I am statically linking my own custom version of libcdio,
so I don't really care much abut API/ABI breakage...
At any rate, since I have now somewhat made a case against the
introduction of these changes, I certainly wouldn't mind seeing other
people chiming in, who instead might offer some good reasons to want to
see these changes in.
After all, now that we have this massive effort, it would certainly feel
like kind of a waste not to use it...
Regards,
/Pete