This is the mail archive of the mailing list for the elfutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Trust AC_SYS_LARGEFILE to provide large file support

On 10/09/2015 04:17 AM, Mark Wielaard wrote:
> On Thu, 2015-10-08 at 13:05 -0700, Josh Stone wrote:
>> AC_SYS_LARGEFILE defines _FILE_OFFSET_BITS in config.h if needed for
>> LFS, and this automatically maps things like open to open64.  But quite
>> a few places used explicit 64-bit names, which won't work on platforms
>> like FreeBSD where off_t is always 64-bit and there are no foo64 names.
>> It's better to just trust that AC_SYS_LARGEFILE is doing it correctly.
>> But we can verify this too, as some file could easily forget to include
>> config.h.  The new tests/ checks all build targets
>> against lfs-symbols (taken from lintian) to make sure everything was
>> implicitly mapped to 64-bit variants when _FILE_OFFSET_BITS is set.
> Very nice cleanup and a clever testscase.
> Please do check it in.

Thanks!  Will do.

> Two questions though (they can be dealt with separately):
>> diff --git a/libdwfl/linux-kernel-modules.c b/libdwfl/linux-kernel-modules.c
>> index dafe893cbda5..38b5170a1a81 100644
>> --- a/libdwfl/linux-kernel-modules.c
>> +++ b/libdwfl/linux-kernel-modules.c
>> @@ -44,6 +44,13 @@
>>  #include <fcntl.h>
>>  #include <unistd.h>
>> +/* Since fts.h is included before config.h, its indirect inclusions may not
>> +   give us the right LFS aliases of these functions, so map them manually.  */
>> +#ifdef _FILE_OFFSET_BITS
>> +#define open open64
>> +#define fopen fopen64
>> +#endif
>> +
> What is the story behind fts.h? Is its usage not off_t 64bit clean?


Of course it does support 64-bit off_t, as it must for native 64-bit
platforms.  I guess the problem is just that it doesn't have the
duplicated interfaces for 32-bit with/without _FILE_OFFSET_BITS.

Note that struct _ftsent contains a struct stat.  So it would need
versions with either stat or stat64, and all of the functions updated to
swing both ways too.

There is the similar ftw.h which has full LFS, but the FTW_SKIP_SUBTREE
action is a GNU extension.  We now use FTS_SKIP to prune the "source"
directory from kernel module walks.  We wouldn't be able to do this with
ftw on FreeBSD, for example, but it's for Linux modules anyway so maybe
we don't care.

Want to convert to ftw and accept the lack of skipping as a fallback?
(There may be other fts/ftw differences that I haven't noticed yet.)

> And what should we do about the usage of loff_t in the public libelf.h
> API? We cannot just change those to off_t since we require 64bit
> offsets. And we don't want a different ABI depending on how libelf.h is
> included (with/without _FILE_OFFSET_BITS).

I have another patch that I'll send which makes an AC_SUBST for this.
Let it remain loff_t whereever that's available, else make it off_t.
This way the installed header still won't depend on configuration.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]