Skip to contentSkip to conversation

getrlimit/getrlimit03: add configure-time check for struct ulimit64

PATCH Superseded Greg Hackmann 2 participants 8 messages Dec 3, 2018 – Dec 5, 2018

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Greg Hackmann

author
#0 lore raw

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

The kernel's UAPI headers export a definition of struct ulimit64 which can conflict with the open-coded one in getrlimit03. Fix this by moving getrlimit03's definition behind a configure-time check.

Signed-off-by Greg Hackmann <[email protected]>
---
 configure.ac                                      |  1 +
 m4/ltp-rlimit64.m4                                | 10 ++++++++++
 testcases/kernel/syscalls/getrlimit/getrlimit03.c |  2 ++
 3 files changed, 13 insertions(+)
 create mode 100644 m4/ltp-rlimit64.m4
configure.ac +1 -0
index e81d2add5..caea34462 100644
@@ -228,6 +228,7 @@ LTP_CHECK_UNAME_DOMAINNAME
 LTP_CHECK_X_TABLES
 LTP_CHECK_ATOMIC_MEMORY_MODEL
 LTP_CHECK_TPACKET_V3
+LTP_CHECK_RLIMIT64
 LTP_DETECT_HOST_CPU
 LTP_CHECK_PERF_EVENT
m4/ltp-rlimit64.m4 new file +10 -0
new file mode 100644
index 000000000..f2e6c526a
@@ -0,0 +1,10 @@
+dnl SPDX-License-Identifier: GPL-2.0-or-later
+dnl Copyright (c) 2018 Google, Inc.
+
+AC_DEFUN([LTP_CHECK_RLIMIT64],[
+AC_CHECK_TYPES([struct rlimit64],,,[
+#include <sys/time.h>
+#include <sys/resource.h>
+])
+])
+
testcases/kernel/syscalls/getrlimit/getrlimit03.c +2 -0
index 9ff28acb6..376ef7241 100644
@@ -44,10 +44,12 @@
 #define __NR_getrlimit_ulong_str	"__NR_getrlimit"
 #endif
 
+#ifndef HAVE_STRUCT_RLIMIT64
 struct rlimit64 {
 	uint64_t rlim_cur;
 	uint64_t rlim_max;
 };
+#endif
 const uint64_t RLIM_INFINITY_U64 = UINT64_MAX;
 
 static int getrlimit_u64(int resource, struct rlimit64 *rlim)
Signature · 1 line
2.20.0.rc1.387.gf8505762e3-goog
author
#1 lore raw

PATCH 2/2 getrlimit/getrlimit03: add test to runtest/syscalls

When I added this test, I forgot to add it to runtest/syscalls. Let's fix that.

Signed-off-by Greg Hackmann <[email protected]>
---
 runtest/syscalls | 1 +
 1 file changed, 1 insertion(+)
runtest/syscalls +1 -0
index ac1d2d2cd..34b47f36b 100644
@@ -439,6 +439,7 @@ getresuid03 getresuid03
 
 getrlimit01 getrlimit01
 getrlimit02 getrlimit02
+getrlimit03 getrlimit03
 
 get_mempolicy01 get_mempolicy01
 get_robust_list01 get_robust_list01
Signature · 1 line
2.20.0.rc1.387.gf8505762e3-goog
#2 lore raw

PATCH 2/2 getrlimit/getrlimit03: add test to runtest/syscalls

Hi Greg,

Quoted 4 lines

When I added this test, I forgot to add it to runtest/syscalls. Let's fix that.

Signed-off-by Greg Hackmann <[email protected]>

Thanks for your patch, pushed (with slightly changed commit message).

Kind regards,
Petr

#3 lore raw

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

In reply to Greg Hackmann

Hi Greg,

Reviewed-by Petr Vorel <[email protected]>

m4/ltp-rlimit64.m4 line 9

Quoted 11 lines

+++ b/m4/ltp-rlimit64.m4

m4/ltp-rlimit64.m4 new file +9 -0
@@ -0,0 +1,10 @@
+dnl SPDX-License-Identifier: GPL-2.0-or-later
+dnl Copyright (c) 2018 Google, Inc.
+
+AC_DEFUN([LTP_CHECK_RLIMIT64],[
+AC_CHECK_TYPES([struct rlimit64],,,[
+#include <sys/time.h>
+#include <sys/resource.h>
+])
+])

Glibc and uclibc-ng define struct rlimit64 only #ifdef __USE_LARGEFILE64.
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.

Kind regards,
Petr

author
#4 lore raw

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

Quoted 1 line · Petr Vorel

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.

Quoted 7 lines

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.

Kind regards,
Petr

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?

#5 lore raw

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

Hi Greg,

Quoted 5 lines · Petr Vorel
Quoted 1 line

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>).

Quoted 8 lines
Quoted 2 lines

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:

ltp-rlimit64.m4 +1 -1
index 6513e65e5..dccb40188 100644
@@ -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>
 ])
 ])
kernel/syscalls/getrlimit/Makefile +2 -0
index bd617d806..4a776e7b1 100644
@@ -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

author
#6 lore raw

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

Quoted 12 lines · Petr Vorel

Hi Greg,

Quoted 5 lines · Petr Vorel
Quoted 1 line

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.

Quoted 45 lines
Quoted 8 lines
Quoted 2 lines

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:

ltp-rlimit64.m4 +1 -1
index 6513e65e5..dccb40188 100644
@@ -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>
  ])
  ])
kernel/syscalls/getrlimit/Makefile +2 -0
index bd617d806..4a776e7b1 100644
@@ -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
Quoted 5 lines

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.

#7 lore raw

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

Hi Greg,

Quoted 2 lines

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

Thanks for info.

Quoted 5 lines

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.

You answer yourself - RLIM_NLIMITS and other rlimit-related constants are needed.

+ using <linux/resource.h> conflicts with other <time.h> usage in LTP library headers.

In file included from /usr/include/time.h:48,

                 from ../../../../include/safe_file_ops_fn.h:22,
                 from ../../../../include/tst_safe_file_ops.h:27,
                 from ../../../../include/tst_test.h:87,
                 from getrlimit03.c:29:

/usr/include/bits/types/struct_itimerspec.h: At top level:
/usr/include/bits/types/struct_itimerspec.h:8:8: error: redefinition of ‘struct itimerspec’

Kind regards,
Petr