[LTP] [PATCH 1/2] getrlimit/getrlimit03: add configure-time check for struct ulimit64

Greg Hackmann
2018-12-04T17:34:28Z

On 12/04/2018 01:01 AM, Petr Vorel wrote:
> Hi Greg,
>
>> On 12/03/2018 01:25 PM, Petr Vorel wrote:
>>> Glibc and uclibc-ng define struct rlimit64 only #ifdef __USE_LARGEFILE64.
>
>> Makes sense. I ran into this issue with bionic, which doesn't require
>> -D_LARGEFILE64_SOURCE for these kinds of definitions.
> Out of curiosity: how do you use it on bionic? Looking into git, struct rlimit64
> is defined in libc/kernel/uapi/linux/resource.h (generated file form kernel
> headers), not in libc/include/sys/resource.h. And you include in getrlimit03.c
> <sys/resource.h> (not <linux/resource.h>).
>

bionic prefers to grab kernel-facing definitions directly from the UAPI
headers, so sys/resource.h includes linux/resource.h.

I could switch the test to explicitly use linux/resource.h, if you feel
that makes more sense. I originally used sys/resource.h to get all the
other rlimit-related constants. I actually didn't realize that the
kernel already exported a struct rlimit64 definition that we could use
in place of open-coding something.

>>> Would it make sense to change autoconf test to AC_COMPILE_IFELSE and pass -D_LARGEFILE64_SOURCE to it?
>>> + Use it in Makefile, of course.
>
>
>> I'm honestly not that familiar with autotools, so I'm not sure I follow
>> this. Are you suggesting that we assume struct rlimit64 is defined
>> (possibly conditionally on -D_LARGEFILE64_SOURCE), and we really ought to
>> probe whether -D_LARGEFILE64_SOURCE is required to make it visible?
> I propose to define _LARGEFILE64_SOURCE anyway, as it allows us to use structures on
> glibc[1] and uclibc{,-ng} while it does not harm musl and bionic.
> your way to check for it before using it makes sense. I just pointed out,
>
> + <sys/time.h> is no t needed in LTP_CHECK_RLIMIT64.
>
> So I propose following changes to your patch:
>
> diff --git m4/ltp-rlimit64.m4 m4/ltp-rlimit64.m4
> index 6513e65e5..dccb40188 100644
> --- m4/ltp-rlimit64.m4
> +++ m4/ltp-rlimit64.m4
> @@ -3,7 +3,7 @@ dnl Copyright (c) 2018 Google, Inc.
>
> AC_DEFUN([LTP_CHECK_RLIMIT64],[
> AC_CHECK_TYPES([struct rlimit64],,,[
> -#include <sys/time.h>
> +#define _LARGEFILE64_SOURCE
> #include <sys/resource.h>
> ])
> ])
> diff --git testcases/kernel/syscalls/getrlimit/Makefile testcases/kernel/syscalls/getrlimit/Makefile
> index bd617d806..4a776e7b1 100644
> --- testcases/kernel/syscalls/getrlimit/Makefile
> +++ testcases/kernel/syscalls/getrlimit/Makefile
> @@ -18,6 +18,8 @@
>
> top_srcdir ?= ../../../..
>
> +getrlimit03: CFLAGS += -D_LARGEFILE64_SOURCE
> +
> include $(top_srcdir)/include/mk/testcases.mk
>
> include $(top_srcdir)/include/mk/generic_leaf_target.mk
>
>
>
> Kind regards,
> Petr
>
> [1] https://sourceware.org/git/?p=glibc.git;a=blob;f=bits/resource.h;h=7693d20fed284ee1bae5cba8884da319f58e262e;hb=HEAD#l112
>

Sounds reasonable. Thanks for your help here.