Re: [Libcdio-devel] udf_read_sectors cannot handle offsets past 2 GB

[Rocky Bernstein]

On Mon, Jan 23, 2012 at 3:50 PM, Pete Batard <address@hidden> wrote:

> With the previous fix applied, the next issue we are facing is in
> udf_read_sectors() and more specifically the fact that it uses 32 bit
> values when ran on 32 bit systems, for i_byte_offset.
>
> The i_byte_offset value is used with SEEK_SET, so it should always be
> positive, however if you modify the code to have something like:
>
>  i_byte_offset = (i_start * UDF_BLOCKSIZE);
>  if (i_byte_offset < 0) {
>    cdio_warn("i_byte_offset = %d (i_start = %d)",
>      i_byte_offset, i_start);
>    return DRIVER_OP_ERROR;
>  }
>
> You end up with:
>
> Extracting: /mnt/data/test//sources/**install.wim
> ++ WARN: i_byte_offset = -2147483648 (i_start = 1048576)
>  Error reading UDF file /sources/install.wim
> --DEBUG: closed source...
>
>
> Now, the problem we are facing when trying to address this bug is we are
> feeding i_byte_offset to cdio_stream_seek() which in turn calls the seek()
> function from cdio_stream of type:
>
>  typedef driver_return_code_t(*cdio_**data_seek_t)
>               (void *user_data, long offset, int whence);
>
> Unfortunately, this definition, as well as its actual implementation in
> _cdio_stdio.c follows POSIX's fseek() and uses a long for the offset. This
> means checkmate on 32 bit platforms as long as fseek is used.
>
> In other words, the current libcdio stream implementation is limited to 2
> GB streams on 32 bit platforms...
>
> My proposal then would be to switch to fseeko [1], which takes an off_t,
> as well as redefine cdio_data_seek_t to also take an off_t instead of a
> long. With the ability to set off_t to 64 bit for large file support, this
> should do the trick. However this is expected to break any libcdio
> application that implements their own stream using a seek() that follows
> the fseek prototype. Currently, I have no idea if this is a valid concern
> or not.
>
> On the other hand, there exists an AC_FUNC_FSEEKO autoconf directive to
> check fseeko support, so we should at least be able to implement a fallback
> to fseek if fseeko is not available, with the expectation that this will
> only occur on 32 bit platforms so no casting issue.
>
> Does the above look like a workable approach to fix this issue? Or do you
> see another possibility?
>
> Regards,
>
> /Pete
>
> [1] 
> http://fclose.com/p/linux/man/**3p-fseeko/<http://fclose.com/p/linux/man/3p-fseeko/>
>
>
>
>
As you have subsequently noted, libcdio does try to detect and set
_FILE_OFFSET_BITS and _LARGEFILE_SOURCE (via AC_SYS_LARGEFILE)

I hadn't known about fseeko or AC_FUNC_SEEKO.

I don't think believe anyone has ever tried to customize or create a
cdio_stream. And even so, the code is painted in a corner now. So I think
it okay for the next version to be a little incompatible.

We can bump the version number non-linearly to suggest a bigger change, and
of course the libcdio shared library number can reflect a change in
API/ABI.

Stuff like this has happened several times in the past. And although nobody
likes it, I don't see a viable alternative.


But as before, I welcome others to weigh in.

Projects and distributions can always stay with previous releases of
libcdio, and often they do for a while simply because so many things have
dependencies on libraries. The dependencies  tends to lock libraries down
until one is prepared for massive updates.

. . .

Rereading the above, I have probably painted to gloomy a picture. If I
understand things correctly, even with the proposed change, one can set
preprocessor symbols to do the same things as we currently do.

And so I did want to say that ultimately there should be configure flags
that allow people to more directly enable or disable fseeko and largefile
support.