* [LTP] [PATCH 1/2] getrlimit/getrlimit03: add configure-time check for struct ulimit64
@ 2018-12-03 19:01 Greg Hackmann
2018-12-03 19:01 ` [LTP] [PATCH 2/2] getrlimit/getrlimit03: add test to runtest/syscalls Greg Hackmann
2018-12-03 21:25 ` [LTP] [PATCH 1/2] getrlimit/getrlimit03: add configure-time check for struct ulimit64 Petr Vorel
0 siblings, 2 replies; 8+ messages in thread
From: Greg Hackmann @ 2018-12-03 19:01 UTC (permalink / raw)
To: ltp
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 <ghackmann@google.com>
---
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
diff --git a/configure.ac b/configure.ac
index e81d2add5..caea34462 100644
--- a/configure.ac
+++ b/configure.ac
@@ -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
diff --git a/m4/ltp-rlimit64.m4 b/m4/ltp-rlimit64.m4
new file mode 100644
index 000000000..f2e6c526a
--- /dev/null
+++ b/m4/ltp-rlimit64.m4
@@ -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>
+])
+])
+
diff --git a/testcases/kernel/syscalls/getrlimit/getrlimit03.c b/testcases/kernel/syscalls/getrlimit/getrlimit03.c
index 9ff28acb6..376ef7241 100644
--- a/testcases/kernel/syscalls/getrlimit/getrlimit03.c
+++ b/testcases/kernel/syscalls/getrlimit/getrlimit03.c
@@ -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)
--
2.20.0.rc1.387.gf8505762e3-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH 2/2] getrlimit/getrlimit03: add test to runtest/syscalls
2018-12-03 19:01 [LTP] [PATCH 1/2] getrlimit/getrlimit03: add configure-time check for struct ulimit64 Greg Hackmann
@ 2018-12-03 19:01 ` Greg Hackmann
2018-12-03 20:34 ` Petr Vorel
2018-12-03 21:25 ` [LTP] [PATCH 1/2] getrlimit/getrlimit03: add configure-time check for struct ulimit64 Petr Vorel
1 sibling, 1 reply; 8+ messages in thread
From: Greg Hackmann @ 2018-12-03 19:01 UTC (permalink / raw)
To: ltp
When I added this test, I forgot to add it to runtest/syscalls. Let's
fix that.
Signed-off-by: Greg Hackmann <ghackmann@google.com>
---
runtest/syscalls | 1 +
1 file changed, 1 insertion(+)
diff --git a/runtest/syscalls b/runtest/syscalls
index ac1d2d2cd..34b47f36b 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -439,6 +439,7 @@ getresuid03 getresuid03
getrlimit01 getrlimit01
getrlimit02 getrlimit02
+getrlimit03 getrlimit03
get_mempolicy01 get_mempolicy01
get_robust_list01 get_robust_list01
--
2.20.0.rc1.387.gf8505762e3-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH 2/2] getrlimit/getrlimit03: add test to runtest/syscalls
2018-12-03 19:01 ` [LTP] [PATCH 2/2] getrlimit/getrlimit03: add test to runtest/syscalls Greg Hackmann
@ 2018-12-03 20:34 ` Petr Vorel
0 siblings, 0 replies; 8+ messages in thread
From: Petr Vorel @ 2018-12-03 20:34 UTC (permalink / raw)
To: ltp
Hi Greg,
> When I added this test, I forgot to add it to runtest/syscalls. Let's
> fix that.
> Signed-off-by: Greg Hackmann <ghackmann@google.com>
Thanks for your patch, pushed (with slightly changed commit message).
Kind regards,
Petr
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/2] getrlimit/getrlimit03: add configure-time check for struct ulimit64
2018-12-03 19:01 [LTP] [PATCH 1/2] getrlimit/getrlimit03: add configure-time check for struct ulimit64 Greg Hackmann
2018-12-03 19:01 ` [LTP] [PATCH 2/2] getrlimit/getrlimit03: add test to runtest/syscalls Greg Hackmann
@ 2018-12-03 21:25 ` Petr Vorel
2018-12-03 22:20 ` Greg Hackmann
1 sibling, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2018-12-03 21:25 UTC (permalink / raw)
To: ltp
Hi Greg,
Reviewed-by: Petr Vorel <pvorel@suse.cz>
> +++ b/m4/ltp-rlimit64.m4
> @@ -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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/2] getrlimit/getrlimit03: add configure-time check for struct ulimit64
2018-12-03 21:25 ` [LTP] [PATCH 1/2] getrlimit/getrlimit03: add configure-time check for struct ulimit64 Petr Vorel
@ 2018-12-03 22:20 ` Greg Hackmann
2018-12-04 9:01 ` Petr Vorel
0 siblings, 1 reply; 8+ messages in thread
From: Greg Hackmann @ 2018-12-03 22:20 UTC (permalink / raw)
To: ltp
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.
> 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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/2] getrlimit/getrlimit03: add configure-time check for struct ulimit64
2018-12-03 22:20 ` Greg Hackmann
@ 2018-12-04 9:01 ` Petr Vorel
2018-12-04 17:34 ` Greg Hackmann
0 siblings, 1 reply; 8+ messages in thread
From: Petr Vorel @ 2018-12-04 9:01 UTC (permalink / raw)
To: ltp
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>).
> > 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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/2] getrlimit/getrlimit03: add configure-time check for struct ulimit64
2018-12-04 9:01 ` Petr Vorel
@ 2018-12-04 17:34 ` Greg Hackmann
2018-12-05 16:47 ` Petr Vorel
0 siblings, 1 reply; 8+ messages in thread
From: Greg Hackmann @ 2018-12-04 17:34 UTC (permalink / raw)
To: ltp
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH 1/2] getrlimit/getrlimit03: add configure-time check for struct ulimit64
2018-12-04 17:34 ` Greg Hackmann
@ 2018-12-05 16:47 ` Petr Vorel
0 siblings, 0 replies; 8+ messages in thread
From: Petr Vorel @ 2018-12-05 16:47 UTC (permalink / raw)
To: ltp
Hi Greg,
> bionic prefers to grab kernel-facing definitions directly from the UAPI
> headers, so sys/resource.h includes linux/resource.h.
Thanks for info.
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-12-05 16:47 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-03 19:01 [LTP] [PATCH 1/2] getrlimit/getrlimit03: add configure-time check for struct ulimit64 Greg Hackmann
2018-12-03 19:01 ` [LTP] [PATCH 2/2] getrlimit/getrlimit03: add test to runtest/syscalls Greg Hackmann
2018-12-03 20:34 ` Petr Vorel
2018-12-03 21:25 ` [LTP] [PATCH 1/2] getrlimit/getrlimit03: add configure-time check for struct ulimit64 Petr Vorel
2018-12-03 22:20 ` Greg Hackmann
2018-12-04 9:01 ` Petr Vorel
2018-12-04 17:34 ` Greg Hackmann
2018-12-05 16:47 ` Petr Vorel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.