diff mbox

[RFC] Make _FILE_OFFSET_BITS=64 default.

Message ID 26837730.jZzKCPMtpS@vapier
State Rejected
Headers show

Commit Message

Mike Frysinger March 13, 2014, 9:16 a.m. UTC
this one actually builds ... disassembly on x86_64 is sane before/after
-mike

commit edf557a5c81eb55ef49cb7a118e008d96f25e5d6
Author: Mike Frysinger <vapier@gentoo.org>
Date:   Thu Mar 13 05:08:11 2014 -0400

    enable _FILE_OFFSET_BITS=64 by default
    
    We build glibc itself with _FILE_OFFSET_BITS=32 still, but the exported
    headers now have the API defaulting to 64bit for everyone.

Comments

Rich Felker March 13, 2014, 9:20 a.m. UTC | #1
On Thu, Mar 13, 2014 at 05:16:08AM -0400, Mike Frysinger wrote:
> diff --git a/io/Makefile b/io/Makefile
> index 8a6562e..77a1907 100644
> --- a/io/Makefile
> +++ b/io/Makefile
> @@ -99,7 +99,7 @@ CFLAGS-fallocate.c = -fexceptions
>  CFLAGS-fallocate64.c = -fexceptions
>  CFLAGS-sync_file_range.c = -fexceptions
>  
> -CFLAGS-test-stat.c = -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE
> +CFLAGS-test-stat.c = -U_FILE_OFFSET_BITS -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE

If this happens to work, it's purely by chance/redundancy. -U
overrides all -D for the same identifier, even subsequent ones.

Rich
Joseph Myers March 13, 2014, 12:55 p.m. UTC | #2
Proposing patches is clearly very premature here.  First, we should see an 
analysis of the extent to which types affected by such a change appear in 
the ABIs of libraries in GNU/Linux distributions (together with tools 
distributors can use to identify affected libraries so they can manage 
such a change in their distributions, if consensus on such a change were 
reached).
Joseph Myers March 13, 2014, 12:58 p.m. UTC | #3
On Thu, 13 Mar 2014, Rich Felker wrote:

> If this happens to work, it's purely by chance/redundancy. -U
> overrides all -D for the same identifier, even subsequent ones.

We can presume glibc is built with the "gcc" driver, not any separate 
"c99" driver with different precedence rules (see GCC bug 40960).
Mike Frysinger March 13, 2014, 9:28 p.m. UTC | #4
On Thu 13 Mar 2014 05:20:57 Rich Felker wrote:
> On Thu, Mar 13, 2014 at 05:16:08AM -0400, Mike Frysinger wrote:
> > diff --git a/io/Makefile b/io/Makefile
> > index 8a6562e..77a1907 100644
> > --- a/io/Makefile
> > +++ b/io/Makefile
> > @@ -99,7 +99,7 @@ CFLAGS-fallocate.c = -fexceptions
> > 
> >  CFLAGS-fallocate64.c = -fexceptions
> >  CFLAGS-sync_file_range.c = -fexceptions
> > 
> > -CFLAGS-test-stat.c = -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE
> > +CFLAGS-test-stat.c = -U_FILE_OFFSET_BITS -D_FILE_OFFSET_BITS=64
> > -D_LARGEFILE64_SOURCE
>
> If this happens to work, it's purely by chance/redundancy. -U
> overrides all -D for the same identifier, even subsequent ones.

if your compiler driver is unreasonable and isn't sane, then i'm not sure how 
that's our problem.  we're already relying on sane behavior -- see how 
_FORTIFY_SOURCE is used in the tree.
-mike
Mike Frysinger March 13, 2014, 9:30 p.m. UTC | #5
On Thu 13 Mar 2014 12:55:51 Joseph S. Myers wrote:
> Proposing patches is clearly very premature here.  First, we should see an
> analysis of the extent to which types affected by such a change appear in
> the ABIs of libraries in GNU/Linux distributions (together with tools
> distributors can use to identify affected libraries so they can manage
> such a change in their distributions, if consensus on such a change were
> reached).

seems like the discussion has stalled to the same point it has in the past.  
this patch can be used by people to do actual compile tests and see what 
happens.  i'm debating adding it to the next Gentoo glibc version to gather 
some actual data since no one seems to want to move w/out data, but no one 
wants to gather data either.
-mike
Rich Felker March 14, 2014, 5:44 a.m. UTC | #6
On Thu, Mar 13, 2014 at 05:28:22PM -0400, Mike Frysinger wrote:
> On Thu 13 Mar 2014 05:20:57 Rich Felker wrote:
> > On Thu, Mar 13, 2014 at 05:16:08AM -0400, Mike Frysinger wrote:
> > > diff --git a/io/Makefile b/io/Makefile
> > > index 8a6562e..77a1907 100644
> > > --- a/io/Makefile
> > > +++ b/io/Makefile
> > > @@ -99,7 +99,7 @@ CFLAGS-fallocate.c = -fexceptions
> > > 
> > >  CFLAGS-fallocate64.c = -fexceptions
> > >  CFLAGS-sync_file_range.c = -fexceptions
> > > 
> > > -CFLAGS-test-stat.c = -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE
> > > +CFLAGS-test-stat.c = -U_FILE_OFFSET_BITS -D_FILE_OFFSET_BITS=64
> > > -D_LARGEFILE64_SOURCE
> >
> > If this happens to work, it's purely by chance/redundancy. -U
> > overrides all -D for the same identifier, even subsequent ones.
> 
> if your compiler driver is unreasonable and isn't sane, then i'm not sure how 
> that's our problem.  we're already relying on sane behavior -- see how 
> _FORTIFY_SOURCE is used in the tree.
> -mike

I'm merely citing the standards-required behavior. If gcc doesn't
follow this, then I suppose this requires a workaround in the 'c99'
wrapper shell scripts used for providing the c99 utility via gcc. But
that's off-topic.

On the other hand, I still think it is confusing to have the makefile
appearing to rely on gcc-specific behavior that contradicts the
standards, especially when the same behavior would be achieved
unambiguously without the -D (since the patched features.h would treat
-U as implying 64-bit).

Rich
Rich Felker March 14, 2014, 5:45 a.m. UTC | #7
On Thu, Mar 13, 2014 at 05:30:06PM -0400, Mike Frysinger wrote:
> On Thu 13 Mar 2014 12:55:51 Joseph S. Myers wrote:
> > Proposing patches is clearly very premature here.  First, we should see an
> > analysis of the extent to which types affected by such a change appear in
> > the ABIs of libraries in GNU/Linux distributions (together with tools
> > distributors can use to identify affected libraries so they can manage
> > such a change in their distributions, if consensus on such a change were
> > reached).
> 
> seems like the discussion has stalled to the same point it has in the past.  
> this patch can be used by people to do actual compile tests and see what 
> happens.  i'm debating adding it to the next Gentoo glibc version to gather 
> some actual data since no one seems to want to move w/out data, but no one 
> wants to gather data either.
> -mike

Thank you. That would be most appreciated.

Rich
Paul Eggert March 14, 2014, 6:41 a.m. UTC | #8
Mike Frysinger wrote:
> i'm debating adding it to the next Gentoo glibc version to gather
> some actual data since no one seems to want to move w/out data, but no one
> wants to gather data either.

Joseph's message gave me pause, as I worry it's asking for an enormous 
task that can never truly said to be done.  Regardless, I took one step 
to do this, and exhaustively surveyed all libraries installed on my 
Fedora 20 host as part of my routine development for building coreutils, 
GNU Emacs, etc.  I found the following issues:

glibc's own fts.h refuses to compile if __USE_FILE_OFFSET64 is defined.

zlib's zlib.h attempts to work around the glibc problems with 
_FILE_OFFSET_BITS, and its workaround would need adjusting if glibc 
changed the default.

Several libraries use APIs that accept affected data types.  These 
libraries are all compiled with _FILE_OFFSET_BITS #defined to be 64, so 
the proposed change would be beneficial as it would fix any applications 
that are currently mistakenly compiled without defining 
_FILE_OFFSET_BITS to be 64.  These libraries are:

* fontconfig's fontconfig/fontconfig.h defines a function 
FcDirCacheLoadFile that accepts struct stat *

* gawk's gawkapi.h defines a type awk_input_buf_t that has a struct stat.

* glib's glib-2.0/glib/gstdio.h defines g_stat, g_lstat, GStatBuf, all 
of which use struct stat.

* Berkeley DB's libdb/db.h defines db_env_set_func_ftruncate, 
db_env_set_func_pread, db_env_set_func_pwrite, db_env_set_func_seek, all 
functions that accept functions that accept off_t arguments.

* libselinux's selinux/selinux.h defines a function 
matchpathcon_filespec_add that accepts an ino_t.

Here are the libraries I looked at as part of my inspection (these are 
Fedora 20 package names).

GConf2 ImageMagick OpenEXR alsa-lib at-spi2-atk atk bzip2 cairo 
cairo-gobject dbus dbus-glib expat fontconfig freetype gdbm gdk-pixbuf2 
ghostscript giflib glib2 glibc gnustep-base gnustep-gui gnutls gpm gtk3 
harfbuzz ilmbase jasper java-1.7.0-openjdk keyutils-libs krb5 lcms2 
libICE libSM libX11 libXau libXaw libXcomposite libXcursor libXdamage 
libXext libXfixes libXft libXi libXinerama libXmu libXpm libXrandr 
libXrender libXt libXxf86vm libacl libattr libcom_err libdb libdrm 
libicu libjpeg-turbo liblockfile libotf libpng librsvg2 libselinux 
libsepol libstdc++ libtasn1 libtiff libverto libwayland-client 
libwayland-cursor libwebp libxcb libxkbcommon libxml2 m17n-lib 
mesa-libEGL mesa-libGL mesa-libGLU ncurses openssl p11-kit pango pcre 
perl pixman systemtap-sdt valgrind wayland xorg-x11-proto xz zlib
Russ Allbery March 14, 2014, 6:55 a.m. UTC | #9
Mike Frysinger <vapier@gentoo.org> writes:

> i'm debating adding it to the next Gentoo glibc version to gather some
> actual data since no one seems to want to move w/out data, but no one
> wants to gather data either.

The change will definitely break INN, which uses off_t in on-disk data
structures.  (A bad design decision made years ago and never properly
expunged.)  That said, it's already the case that INN built with
large-file support and without uses incompatible data structures and
whether to do so is a standard compile-time option, so distributions are
already aware of this problem and are already dealing with it in some
fashion, and INN could be changed to undefine the relevant macros if
configured to *not* have large file support (instead of what it does
now).
Mike Frysinger March 14, 2014, 7:18 a.m. UTC | #10
On Fri 14 Mar 2014 01:44:30 Rich Felker wrote:
> On Thu, Mar 13, 2014 at 05:28:22PM -0400, Mike Frysinger wrote:
> > On Thu 13 Mar 2014 05:20:57 Rich Felker wrote:
> > > On Thu, Mar 13, 2014 at 05:16:08AM -0400, Mike Frysinger wrote:
> > > > diff --git a/io/Makefile b/io/Makefile
> > > > index 8a6562e..77a1907 100644
> > > > --- a/io/Makefile
> > > > +++ b/io/Makefile
> > > > @@ -99,7 +99,7 @@ CFLAGS-fallocate.c = -fexceptions
> > > > 
> > > >  CFLAGS-fallocate64.c = -fexceptions
> > > >  CFLAGS-sync_file_range.c = -fexceptions
> > > > 
> > > > -CFLAGS-test-stat.c = -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE
> > > > +CFLAGS-test-stat.c = -U_FILE_OFFSET_BITS -D_FILE_OFFSET_BITS=64
> > > > -D_LARGEFILE64_SOURCE
> > > 
> > > If this happens to work, it's purely by chance/redundancy. -U
> > > overrides all -D for the same identifier, even subsequent ones.
> > 
> > if your compiler driver is unreasonable and isn't sane, then i'm not sure
> > how that's our problem.  we're already relying on sane behavior -- see
> > how _FORTIFY_SOURCE is used in the tree.
> > -mike
> 
> I'm merely citing the standards-required behavior. If gcc doesn't
> follow this, then I suppose this requires a workaround in the 'c99'
> wrapper shell scripts used for providing the c99 utility via gcc. But
> that's off-topic.
> 
> On the other hand, I still think it is confusing to have the makefile
> appearing to rely on gcc-specific behavior that contradicts the
> standards, especially when the same behavior would be achieved
> unambiguously without the -D (since the patched features.h would treat
> -U as implying 64-bit).

because elsewhere in the patch i add -D_FILE_OFFSET_BITS=32 to the global 
CPPFLAGS list so that glibc gets the environment it expects -- 32bit by 
default.  you could probably argue that glibc should be changed to expect the 
64bit API by default, but that would require a lot of careful surgery through 
out the tree and testing by every arch ... something we can/should do, but not 
at the expense of moving forward.
-mike
Joseph Myers March 14, 2014, 2:13 p.m. UTC | #11
On Thu, 13 Mar 2014, Paul Eggert wrote:

> Joseph's message gave me pause, as I worry it's asking for an enormous task
> that can never truly said to be done.  Regardless, I took one step to do this,
> and exhaustively surveyed all libraries installed on my Fedora 20 host as part
> of my routine development for building coreutils, GNU Emacs, etc.  I found the
> following issues:

Enumerating all affected types and grepping the headers installed by all 
development packages in a distribution (documenting the details of what 
was done, so distributors can repeat it for other distributions) - not 
just those installed on a particular system - would seem a reasonable 
approach.

> glibc's own fts.h refuses to compile if __USE_FILE_OFFSET64 is defined.

The lack of a 64-bit version of some of glibc's own interfaces would seem 
to be a fairly critical issue with any change to the default; before 
changing the default, either a 64-bit version of those interfaces would 
need adding, or those interfaces would all need to be made into compat 
symbols for existing binaries only and the header removed.
Rich Felker March 14, 2014, 2:47 p.m. UTC | #12
On Fri, Mar 14, 2014 at 03:18:22AM -0400, Mike Frysinger wrote:
> On Fri 14 Mar 2014 01:44:30 Rich Felker wrote:
> > On Thu, Mar 13, 2014 at 05:28:22PM -0400, Mike Frysinger wrote:
> > > On Thu 13 Mar 2014 05:20:57 Rich Felker wrote:
> > > > On Thu, Mar 13, 2014 at 05:16:08AM -0400, Mike Frysinger wrote:
> > > > > diff --git a/io/Makefile b/io/Makefile
> > > > > index 8a6562e..77a1907 100644
> > > > > --- a/io/Makefile
> > > > > +++ b/io/Makefile
> > > > > @@ -99,7 +99,7 @@ CFLAGS-fallocate.c = -fexceptions
> > > > > 
> > > > >  CFLAGS-fallocate64.c = -fexceptions
> > > > >  CFLAGS-sync_file_range.c = -fexceptions
> > > > > 
> > > > > -CFLAGS-test-stat.c = -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE
> > > > > +CFLAGS-test-stat.c = -U_FILE_OFFSET_BITS -D_FILE_OFFSET_BITS=64
> > > > > -D_LARGEFILE64_SOURCE
> > > > 
> > > > If this happens to work, it's purely by chance/redundancy. -U
> > > > overrides all -D for the same identifier, even subsequent ones.
> > > 
> > > if your compiler driver is unreasonable and isn't sane, then i'm not sure
> > > how that's our problem.  we're already relying on sane behavior -- see
> > > how _FORTIFY_SOURCE is used in the tree.
> > > -mike
> > 
> > I'm merely citing the standards-required behavior. If gcc doesn't
> > follow this, then I suppose this requires a workaround in the 'c99'
> > wrapper shell scripts used for providing the c99 utility via gcc. But
> > that's off-topic.
> > 
> > On the other hand, I still think it is confusing to have the makefile
> > appearing to rely on gcc-specific behavior that contradicts the
> > standards, especially when the same behavior would be achieved
> > unambiguously without the -D (since the patched features.h would treat
> > -U as implying 64-bit).
> 
> because elsewhere in the patch i add -D_FILE_OFFSET_BITS=32 to the global 
> CPPFLAGS list so that glibc gets the environment it expects -- 32bit by 
> default.  you could probably argue that glibc should be changed to expect the 
> 64bit API by default, but that would require a lot of careful surgery through 
> out the tree and testing by every arch ... something we can/should do, but not 
> at the expense of moving forward.
> -mike

What I was trying to say is that adding -U_FILE_OFFSET_BITS will
suppress the -D_FILE_OFFSET_BITS=32 and allow the default in
features.h to do its thing.

Rich
Rich Felker March 14, 2014, 6:09 p.m. UTC | #13
On Thu, Mar 13, 2014 at 11:55:55PM -0700, Russ Allbery wrote:
> Mike Frysinger <vapier@gentoo.org> writes:
> 
> > i'm debating adding it to the next Gentoo glibc version to gather some
> > actual data since no one seems to want to move w/out data, but no one
> > wants to gather data either.
> 
> The change will definitely break INN, which uses off_t in on-disk data
> structures.  (A bad design decision made years ago and never properly
> expunged.)  That said, it's already the case that INN built with
> large-file support and without uses incompatible data structures and
> whether to do so is a standard compile-time option, so distributions are
> already aware of this problem and are already dealing with it in some
> fashion, and INN could be changed to undefine the relevant macros if
> configured to *not* have large file support (instead of what it does
> now).

Do you have a link to INN? The name isn't exactly googlable... In any
case, they probably should just deprecate the non-64-bit build since
other things are likely broken too (e.g. if they use stat) and add
automatic file format conversion if they don't already have it..

Rich
Rich Felker March 14, 2014, 6:18 p.m. UTC | #14
On Thu, Mar 13, 2014 at 11:41:48PM -0700, Paul Eggert wrote:
> Mike Frysinger wrote:
> >i'm debating adding it to the next Gentoo glibc version to gather
> >some actual data since no one seems to want to move w/out data, but no one
> >wants to gather data either.
> 
> Joseph's message gave me pause, as I worry it's asking for an
> enormous task that can never truly said to be done.  Regardless, I
> took one step to do this, and exhaustively surveyed all libraries
> installed on my Fedora 20 host as part of my routine development for
> building coreutils, GNU Emacs, etc.  I found the following issues:
> 
> glibc's own fts.h refuses to compile if __USE_FILE_OFFSET64 is defined.

Known issue:
https://sourceware.org/bugzilla/show_bug.cgi?id=15838

As stated there, my preference would be deprecation this whole API.
Applications which need it can get a _working_ (unlike the glibc copy)
version of this functionality from third-party sources. Gnulib and BSD
both have working versions that don't have the 32-bit limitation.

> zlib's zlib.h attempts to work around the glibc problems with
> _FILE_OFFSET_BITS, and its workaround would need adjusting if glibc
> changed the default.

Can you explain in more detail?

> Several libraries use APIs that accept affected data types.  These
> libraries are all compiled with _FILE_OFFSET_BITS #defined to be 64,
> so the proposed change would be beneficial as it would fix any
> applications that are currently mistakenly compiled without defining
> _FILE_OFFSET_BITS to be 64.  These libraries are:

Exactly. That's what I've been saying all along. Once the default has
changed, this issue will gradually go away and hopefully never return.

Rich
Russ Allbery March 14, 2014, 6:23 p.m. UTC | #15
Rich Felker <dalias@aerifal.cx> writes:

> Do you have a link to INN? The name isn't exactly googlable... In any
> case, they probably should just deprecate the non-64-bit build since
> other things are likely broken too (e.g. if they use stat) and add
> automatic file format conversion if they don't already have it..

http://www.eyrie.org/~eagle/software/inn/ has the most resources at the
moment, although the official home page is at ISC.

The automatic format conversion is the hard part that no one has written
yet.  The history file format (which uses the old dbz data format), the
tradindexed overview backend, and the timecaf storage backend are
affected.  CNFS was written properly and is not affected.  For history and
tradindexed, only the indices are affected and those are relatively easy
to rebuild, so those aren't a huge problem for a one-time conversion.
timecaf is more of a problem and really needs a conversion tool.

I completely agree with you that the non-64-bit build mode should be
deprecated.  I even wrote the following comment in configure.ac years and
years ago, before I ran out of time to work on INN:

dnl Whether to use the OS flags to enable large file support.  Ideally this
dnl should just always be turned on if possible and the various parts of INN
dnl that read off_t's from disk should adjust somehow to the size, but INN
dnl isn't there yet.  Currently tagged hash doesn't work with large file
dnl support due to assumptions about the size of off_t.

(I'm not sure anyone actually uses the tagged hash history storage method
any more.)

Debian already has separate inn2 and inn2-lfs packages and will have to do
some sort of migration because Perl is built with LFS support and there's
no way for the non-64-bit INN to link with the LFS libperl (the only
libperl available in Debian) without all sorts of weird problems, since
Perl sizes several of its ABI data structures based on the size of off_t.
Paul Eggert March 14, 2014, 9:30 p.m. UTC | #16
On 03/14/2014 11:18 AM, Rich Felker wrote:
>> >zlib's zlib.h attempts to work around the glibc problems with
>> >_FILE_OFFSET_BITS, and its workaround would need adjusting if glibc
>> >changed the default.
> Can you explain in more detail?

It's pretty obscure, and I expect we don't need to worry about it.  As I 
understand it, the problem affects only applications that use 
-D_FILE_OFFSET_BITS=32.  I don't know of any (who would want to do 
that?!), but there it is.  zconf.h does this:

#if defined(_LFS64_LARGEFILE) && _LFS64_LARGEFILE-0
#  define Z_LFS64
#endif
...
#if defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS-0 == 64 && 
defined(Z_LFS64)
#  define Z_WANT64
#endif

and Z_WANT64's only use is in zlib.h, in the code at the end of this 
email.  This causes zlib to use a workaroundsomething like glibc's when 
_FILE_OFFSET_BITSis explicitly defined to be 64.  zlib itself is 
compiled with no _FILE_OFFSET_BITS setting (it uses z_off64_t, lseek64 
etc. internally), so if glibc changes the _FILE_OFFSET_BITS default to 
64, zlib would then support only 64-bit off_t, and zlib's workaround 
would break programs compiled with -D_FILE_OFFSET_BITS=32. If zlib 
wanted to continue to support 32-bit-only off_t, it'd need to compile 
itself with -D_FILE_OFFSET_BITS=32, and tweak zconf.h and zlib.h a bit.  
My guess is that zlib wouldn't bother.  I know zlib's maintainer and so 
could ask him, if there's interest.

#if !defined(ZLIB_INTERNAL) && defined(Z_WANT64)
#  ifdef Z_PREFIX_SET
#    define z_gzopen z_gzopen64
#    define z_gzseek z_gzseek64
#    define z_gztell z_gztell64
#    define z_gzoffset z_gzoffset64
#    define z_adler32_combine z_adler32_combine64
#    define z_crc32_combine z_crc32_combine64
#  else
#    define gzopen gzopen64
#    define gzseek gzseek64
#    define gztell gztell64
#    define gzoffset gzoffset64
#    define adler32_combine adler32_combine64
#    define crc32_combine crc32_combine64
#  endif
#  ifndef Z_LARGE64
      ZEXTERN gzFile ZEXPORT gzopen64 OF((const char *, const char *));
      ZEXTERN z_off_t ZEXPORT gzseek64 OF((gzFile, z_off_t, int));
      ZEXTERN z_off_t ZEXPORT gztell64 OF((gzFile));
      ZEXTERN z_off_t ZEXPORT gzoffset64 OF((gzFile));
      ZEXTERN uLong ZEXPORT adler32_combine64 OF((uLong, uLong, z_off_t));
      ZEXTERN uLong ZEXPORT crc32_combine64 OF((uLong, uLong, z_off_t));
#  endif
#else
    ZEXTERN gzFile ZEXPORT gzopen OF((const char *, const char *));
    ZEXTERN z_off_t ZEXPORT gzseek OF((gzFile, z_off_t, int));
    ZEXTERN z_off_t ZEXPORT gztell OF((gzFile));
    ZEXTERN z_off_t ZEXPORT gzoffset OF((gzFile));
    ZEXTERN uLong ZEXPORT adler32_combine OF((uLong, uLong, z_off_t));
    ZEXTERN uLong ZEXPORT crc32_combine OF((uLong, uLong, z_off_t));
#endif
Mike Frysinger March 19, 2014, 5:53 a.m. UTC | #17
On Fri 14 Mar 2014 14:18:37 Rich Felker wrote:
> On Thu, Mar 13, 2014 at 11:41:48PM -0700, Paul Eggert wrote:
> > glibc's own fts.h refuses to compile if __USE_FILE_OFFSET64 is defined.
> 
> Known issue:
> https://sourceware.org/bugzilla/show_bug.cgi?id=15838
> 
> As stated there, my preference would be deprecation this whole API.
> Applications which need it can get a _working_ (unlike the glibc copy)
> version of this functionality from third-party sources. Gnulib and BSD
> both have working versions that don't have the 32-bit limitation.

so wouldn't the right answer be to import the latest gnulib version ?

is there a preference for fts over ftw ?  i've never actually used either 
myself.
-mike
Rich Felker March 19, 2014, 6:03 a.m. UTC | #18
On Wed, Mar 19, 2014 at 01:53:11AM -0400, Mike Frysinger wrote:
> On Fri 14 Mar 2014 14:18:37 Rich Felker wrote:
> > On Thu, Mar 13, 2014 at 11:41:48PM -0700, Paul Eggert wrote:
> > > glibc's own fts.h refuses to compile if __USE_FILE_OFFSET64 is defined.
> > 
> > Known issue:
> > https://sourceware.org/bugzilla/show_bug.cgi?id=15838
> > 
> > As stated there, my preference would be deprecation this whole API.
> > Applications which need it can get a _working_ (unlike the glibc copy)
> > version of this functionality from third-party sources. Gnulib and BSD
> > both have working versions that don't have the 32-bit limitation.
> 
> so wouldn't the right answer be to import the latest gnulib version ?

No, because the ABI is incompatible with glibc's interface. The reason
glibc is stuck with the broken one and gnulib isn't is that glibc has
to maintain an ABI while gnulib doesn't.

Of course this could be fixed by adding a new fts64 interface in
glibc; the question is just whether it's better to do this or
deprecate the interface in libc (as it really has no need to be in
libc).

> is there a preference for fts over ftw ?  i've never actually used either 
> myself.

I've never used fts but it looks better than ftw..

Rich
Mike Frysinger March 19, 2014, 6:50 a.m. UTC | #19
On Wed 19 Mar 2014 02:03:11 Rich Felker wrote:
> On Wed, Mar 19, 2014 at 01:53:11AM -0400, Mike Frysinger wrote:
> > On Fri 14 Mar 2014 14:18:37 Rich Felker wrote:
> > > On Thu, Mar 13, 2014 at 11:41:48PM -0700, Paul Eggert wrote:
> > > > glibc's own fts.h refuses to compile if __USE_FILE_OFFSET64 is
> > > > defined.
> > > 
> > > Known issue:
> > > https://sourceware.org/bugzilla/show_bug.cgi?id=15838
> > > 
> > > As stated there, my preference would be deprecation this whole API.
> > > Applications which need it can get a _working_ (unlike the glibc copy)
> > > version of this functionality from third-party sources. Gnulib and BSD
> > > both have working versions that don't have the 32-bit limitation.
> > 
> > so wouldn't the right answer be to import the latest gnulib version ?
> 
> No, because the ABI is incompatible with glibc's interface. The reason
> glibc is stuck with the broken one and gnulib isn't is that glibc has
> to maintain an ABI while gnulib doesn't.

err, no it doesn't.  that's the entire point of versioned symbols.  glibc is 
only restricted in maintaining past ABI compatibility, not being stuck with 
crap for the whole future.

fork the existing fts symbols off and create brand new ones with GLIBC_2.20 
versions and you're done.
-mike
Rich Felker March 19, 2014, 6:56 a.m. UTC | #20
On Wed, Mar 19, 2014 at 02:50:35AM -0400, Mike Frysinger wrote:
> On Wed 19 Mar 2014 02:03:11 Rich Felker wrote:
> > On Wed, Mar 19, 2014 at 01:53:11AM -0400, Mike Frysinger wrote:
> > > On Fri 14 Mar 2014 14:18:37 Rich Felker wrote:
> > > > On Thu, Mar 13, 2014 at 11:41:48PM -0700, Paul Eggert wrote:
> > > > > glibc's own fts.h refuses to compile if __USE_FILE_OFFSET64 is
> > > > > defined.
> > > > 
> > > > Known issue:
> > > > https://sourceware.org/bugzilla/show_bug.cgi?id=15838
> > > > 
> > > > As stated there, my preference would be deprecation this whole API.
> > > > Applications which need it can get a _working_ (unlike the glibc copy)
> > > > version of this functionality from third-party sources. Gnulib and BSD
> > > > both have working versions that don't have the 32-bit limitation.
> > > 
> > > so wouldn't the right answer be to import the latest gnulib version ?
> > 
> > No, because the ABI is incompatible with glibc's interface. The reason
> > glibc is stuck with the broken one and gnulib isn't is that glibc has
> > to maintain an ABI while gnulib doesn't.
> 
> err, no it doesn't.  that's the entire point of versioned symbols.  glibc is 
> only restricted in maintaining past ABI compatibility, not being stuck with 
> crap for the whole future.
> 
> fork the existing fts symbols off and create brand new ones with GLIBC_2.20 
> versions and you're done.

Just doing it that way, the new interfaces would be incompatible with
-D_FILE_OFFSET_BITS=32. If symbol versioning is going to play a role
in solving this problem, I don't think it should be done to just one
odd-ball interface like fts, but as a global solution. That would
entail dropping support for 32-bit off_t, making all of the old 32-bit
functions accessible only via old symbol versions, and putting new
64-bit versions in front of them. This would of course simplify the
headers a lot; all of the redirection mess could be removed. I'm all
in favor of this solution, but it has a snowball's chance in hell of
being accepted by everybody else, so...

Rich
diff mbox

Patch

diff --git a/Makeconfig b/Makeconfig
index 9078b29..b3ca1ee 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -785,7 +785,8 @@  libio-include = -I$(..)libio
 # Note that we can't use -std=* in CPPFLAGS, because it overrides
 # the implicit -lang-asm and breaks cpp behavior for .S files--notably
 # it causes cpp to stop predefining __ASSEMBLER__.
-CPPFLAGS = $(CPPUNDEFS) $(CPPFLAGS-config) $($(subdir)-CPPFLAGS) \
+CPPFLAGS = $(CPPUNDEFS) -D_FILE_OFFSET_BITS=32 \
+	   $(CPPFLAGS-config) $($(subdir)-CPPFLAGS) \
 	   $(+includes) $(defines) \
 	   -include $(..)include/libc-symbols.h $(sysdep-CPPFLAGS) \
 	   $(CPPFLAGS-$(suffix $@)) \
diff --git a/debug/tst-lfschk1.c b/debug/tst-lfschk1.c
index f3e6d47..9f04600 100644
--- a/debug/tst-lfschk1.c
+++ b/debug/tst-lfschk1.c
@@ -1,2 +1,3 @@ 
+#undef _FILE_OFFSET_BITS
 #define _FILE_OFFSET_BITS 64
 #include "tst-chk1.c"
diff --git a/debug/tst-lfschk2.c b/debug/tst-lfschk2.c
index 95d4db1..34910e7 100644
--- a/debug/tst-lfschk2.c
+++ b/debug/tst-lfschk2.c
@@ -1,2 +1,3 @@ 
+#undef _FILE_OFFSET_BITS
 #define _FILE_OFFSET_BITS 64
 #include "tst-chk2.c"
diff --git a/debug/tst-lfschk3.c b/debug/tst-lfschk3.c
index 50a1ae1..7185608 100644
--- a/debug/tst-lfschk3.c
+++ b/debug/tst-lfschk3.c
@@ -1,2 +1,3 @@ 
+#undef _FILE_OFFSET_BITS
 #define _FILE_OFFSET_BITS 64
 #include "tst-chk3.c"
diff --git a/debug/tst-lfschk4.cc b/debug/tst-lfschk4.cc
index f3e6d47..9f04600 100644
--- a/debug/tst-lfschk4.cc
+++ b/debug/tst-lfschk4.cc
@@ -1,2 +1,3 @@ 
+#undef _FILE_OFFSET_BITS
 #define _FILE_OFFSET_BITS 64
 #include "tst-chk1.c"
diff --git a/debug/tst-lfschk5.cc b/debug/tst-lfschk5.cc
index 95d4db1..34910e7 100644
--- a/debug/tst-lfschk5.cc
+++ b/debug/tst-lfschk5.cc
@@ -1,2 +1,3 @@ 
+#undef _FILE_OFFSET_BITS
 #define _FILE_OFFSET_BITS 64
 #include "tst-chk2.c"
diff --git a/debug/tst-lfschk6.cc b/debug/tst-lfschk6.cc
index 50a1ae1..7185608 100644
--- a/debug/tst-lfschk6.cc
+++ b/debug/tst-lfschk6.cc
@@ -1,2 +1,3 @@ 
+#undef _FILE_OFFSET_BITS
 #define _FILE_OFFSET_BITS 64
 #include "tst-chk3.c"
diff --git a/include/features.h b/include/features.h
index c3ed81f..aa12c65 100644
--- a/include/features.h
+++ b/include/features.h
@@ -303,7 +303,7 @@ 
 # define __USE_LARGEFILE64	1
 #endif
 
-#if defined _FILE_OFFSET_BITS && _FILE_OFFSET_BITS == 64
+#if !defined _FILE_OFFSET_BITS || _FILE_OFFSET_BITS == 64
 # define __USE_FILE_OFFSET64	1
 #endif
 
diff --git a/io/Makefile b/io/Makefile
index 8a6562e..77a1907 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -99,7 +99,7 @@  CFLAGS-fallocate.c = -fexceptions
 CFLAGS-fallocate64.c = -fexceptions
 CFLAGS-sync_file_range.c = -fexceptions
 
-CFLAGS-test-stat.c = -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE
+CFLAGS-test-stat.c = -U_FILE_OFFSET_BITS -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE
 CFLAGS-test-lfs.c = -D_LARGEFILE64_SOURCE
 
 test-stat2-ARGS = Makefile . $(objpfx)test-stat2
diff --git a/malloc/memusagestat.c b/malloc/memusagestat.c
index 3e0889e..96d7e28 100644
--- a/malloc/memusagestat.c
+++ b/malloc/memusagestat.c
@@ -16,6 +16,7 @@ 
    You should have received a copy of the GNU General Public License
    along with this program; if not, see <http://www.gnu.org/licenses/>.  */
 
+#undef _FILE_OFFSET_BITS
 #define _FILE_OFFSET_BITS 64
 
 #include <argp.h>