diff mbox

[xfsprogs,01/14] configure: use AC_SYS_LARGEFILE

Message ID ea8cca9bf1dbeb13a788f85f73d6bafbcc374a89.1470555003.git.felix.janda@posteo.de (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Felix Janda Aug. 7, 2016, 5:21 a.m. UTC
The autoconf macro AC_SYS_LARGEFILE defines _FILE_OFFSET_BITS=64
where necessary to ensure that off_t and all interfaces using off_t
are 64bit, even on 32bit systems.

Signed-off-by: Felix Janda <felix.janda@posteo.de>
---
 configure.ac | 2 ++
 1 file changed, 2 insertions(+)

Comments

Christoph Hellwig Aug. 9, 2016, 7:36 a.m. UTC | #1
Good plan:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Does this also error out for libraries that don't support large
off_t at all?  I think that would be helpful to add if not there yet.
Felix Janda Aug. 9, 2016, 5:41 p.m. UTC | #2
Thanks for reviewing the patch series!

> Does this also error out for libraries that don't support large
> off_t at all?  I think that would be helpful to add if not there yet.

I do not quite understand. Do you refer to libraries using libxfs or
the external libraries used by xfsprogs?

For the latter, none of them exports interfaces using off_t. libblkid
has its own blkid_loff_t, which is defined as int64_t.

For the former, patch 13 forces any user of libxfs to enable
transparent LFS, by for example adding AC_SYS_LARGEFILE.

The approach of libblkid is the same as what I was suggesting in a
previous patch, but maybe it is good to break applications using
libxfs and not transparent LFS. For example this seems to be the case
for ceph. It has not enabled transparent LFS but mixes off_t and
off64_t. So it is likely that it has some LFS related runtime bugs on
32bit systems. If the xfs header included the off_t size check,
building ceph on 32bit systems would lead to a less subtle compile
failure.

Felix
Christoph Hellwig Aug. 12, 2016, 2:57 a.m. UTC | #3
On Tue, Aug 09, 2016 at 07:41:05PM +0200, Felix Janda wrote:
> Thanks for reviewing the patch series!
> 
> > Does this also error out for libraries that don't support large
> > off_t at all?  I think that would be helpful to add if not there yet.
> 
> I do not quite understand. Do you refer to libraries using libxfs or
> the external libraries used by xfsprogs?

I meant C libraries, sorry.  E.g. uclibc used to not support LFS
many years ago, although they probably fixed it up by now.
Felix Janda Aug. 12, 2016, 4:54 p.m. UTC | #4
Christoph Hellwig wrote:
> On Tue, Aug 09, 2016 at 07:41:05PM +0200, Felix Janda wrote:
> > Thanks for reviewing the patch series!
> > 
> > > Does this also error out for libraries that don't support large
> > > off_t at all?  I think that would be helpful to add if not there yet.
> > 
> > I do not quite understand. Do you refer to libraries using libxfs or
> > the external libraries used by xfsprogs?
> 
> I meant C libraries, sorry.  E.g. uclibc used to not support LFS
> many years ago, although they probably fixed it up by now.

Thaks for clarifying.

If a libc does not support LFS, with this patch series the build will
fail soon, because of the off_t size check in xfs.h.

The support of transparent LFS in different c libraries on linux seems
to be the following:

glibc: since version 2.2 (2000)
uClibc: since version 0.9.11 (2002)
dietlibc: since version 0.8 (2001)
klibc: AFAICS since beginning only transparent LFS
musl: since beginning only transparent LFS
bionic: In 2015 _FILE_OFFSET_BITS was implemented "mostly"...
newlib: (except on cygwin) does not seem to have support for transparent LFS

Note that LFS can be configured out of uClibc. However its headers
error out when it is configured out and an application sets
_FILE_OFFSET_BITS. (So in the case of xfsprogs it would have errored
out in this situation already earlier.)


So it seems that this patch series breaks newlib support...

Felix
Christoph Hellwig Aug. 12, 2016, 8:23 p.m. UTC | #5
On Fri, Aug 12, 2016 at 06:54:40PM +0200, Felix Janda wrote:
> glibc: since version 2.2 (2000)
> uClibc: since version 0.9.11 (2002)
> dietlibc: since version 0.8 (2001)
> klibc: AFAICS since beginning only transparent LFS
> musl: since beginning only transparent LFS
> bionic: In 2015 _FILE_OFFSET_BITS was implemented "mostly"...
> newlib: (except on cygwin) does not seem to have support for transparent LFS
> 
> Note that LFS can be configured out of uClibc. However its headers
> error out when it is configured out and an application sets
> _FILE_OFFSET_BITS. (So in the case of xfsprogs it would have errored
> out in this situation already earlier.)
> 
> 
> So it seems that this patch series breaks newlib support...

I think that's fine, newlib hasn't every really been a supported
config.  The point I tried to make is that we should aim to error
out during ./configure for this case.
diff mbox

Patch

diff --git a/configure.ac b/configure.ac
index 1bb5fef..8fa96a5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -107,6 +107,8 @@  AC_PACKAGE_UTILITIES(xfsprogs)
 AC_MULTILIB($enable_lib64)
 AC_RT($enable_librt)
 
+AC_SYS_LARGEFILE
+
 AC_PACKAGE_NEED_UUID_H
 AC_PACKAGE_NEED_UUIDCOMPARE