All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] syscalls/mallinfo201: Add a basic test for mallinfo2 when setting 2G size
@ 2021-05-07  7:30 Yang Xu
  2021-05-25 10:56 ` Petr Vorel
  0 siblings, 1 reply; 11+ messages in thread
From: Yang Xu @ 2021-05-07  7:30 UTC (permalink / raw)
  To: ltp

Since these members of mallinfo struct use int data type, it will overflow
when allocating 2G size. mallinfo() is deprecated and we should use mallinfo2()
in the future. So we test mallinfo2 whether works well.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 configure.ac                                  |  1 +
 runtest/syscalls                              |  2 +
 .../syscalls/mallinfo/mallinfo_common.h       | 18 +++++++
 .../kernel/syscalls/mallinfo2/.gitignore      |  1 +
 testcases/kernel/syscalls/mallinfo2/Makefile  |  8 ++++
 .../kernel/syscalls/mallinfo2/mallinfo201.c   | 47 +++++++++++++++++++
 6 files changed, 77 insertions(+)
 create mode 100644 testcases/kernel/syscalls/mallinfo2/.gitignore
 create mode 100644 testcases/kernel/syscalls/mallinfo2/Makefile
 create mode 100644 testcases/kernel/syscalls/mallinfo2/mallinfo201.c

diff --git a/configure.ac b/configure.ac
index 136d82d09..eb675b367 100644
--- a/configure.ac
+++ b/configure.ac
@@ -102,6 +102,7 @@ AC_CHECK_FUNCS_ONCE([ \
     io_uring_enter \
     kcmp \
     mallinfo \
+    mallinfo2 \
     mallopt \
     mkdirat \
     mknodat \
diff --git a/runtest/syscalls b/runtest/syscalls
index aa7fa24dd..ea48efd26 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -693,6 +693,8 @@ lstat02_64 lstat02_64
 mallinfo01 mallinfo01
 mallinfo02 mallinfo02
 
+mallinfo201 mallinfo201
+
 mallopt01 mallopt01
 
 mbind01 mbind01
diff --git a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
index a00cc7a64..e7737b270 100644
--- a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
+++ b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
@@ -28,4 +28,22 @@ static inline void print_mallinfo(const char *msg, struct mallinfo *m)
 }
 #endif
 
+#ifdef HAVE_MALLINFO2
+static inline void print_mallinfo2(const char *msg, struct mallinfo2 *m)
+{
+	tst_res(TINFO, "%s...", msg);
+#define P2(f) tst_res(TINFO, "%s: %ld", #f, m->f)
+	P2(arena);
+	P2(ordblks);
+	P2(smblks);
+	P2(hblks);
+	P2(hblkhd);
+	P2(usmblks);
+	P2(fsmblks);
+	P2(uordblks);
+	P2(fordblks);
+	P2(keepcost);
+}
+#endif
+
 #endif
diff --git a/testcases/kernel/syscalls/mallinfo2/.gitignore b/testcases/kernel/syscalls/mallinfo2/.gitignore
new file mode 100644
index 000000000..848f00130
--- /dev/null
+++ b/testcases/kernel/syscalls/mallinfo2/.gitignore
@@ -0,0 +1 @@
+/mallinfo201
diff --git a/testcases/kernel/syscalls/mallinfo2/Makefile b/testcases/kernel/syscalls/mallinfo2/Makefile
new file mode 100644
index 000000000..044619fb8
--- /dev/null
+++ b/testcases/kernel/syscalls/mallinfo2/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) International Business Machines  Corp., 2001
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/mallinfo2/mallinfo201.c b/testcases/kernel/syscalls/mallinfo2/mallinfo201.c
new file mode 100644
index 000000000..0bc2520a0
--- /dev/null
+++ b/testcases/kernel/syscalls/mallinfo2/mallinfo201.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [DESCRIPTION]
+ *
+ * Basic mallinfo2() test. Test the member of struct mallinfo2
+ * whether overflow when setting 2G size. mallinfo() is deprecated
+ * since the type used for the fields is too small. We should use
+ * mallinfo2() and it was added since glibc 2.33.
+ */
+
+#include "../mallinfo/mallinfo_common.h"
+#include "tst_safe_macros.h"
+
+#ifdef HAVE_MALLINFO2
+
+void test_mallinfo2(void)
+{
+	struct mallinfo2 info;
+	char *buf;
+	size_t size = 2UL * 1024UL * 1024UL * 1024UL;
+
+	buf = malloc(size);
+	if (!buf) {
+		tst_res(TCONF, "Current system can not malloc 2G space, skip it");
+		return;
+	}
+	info = mallinfo2();
+	if (info.hblkhd < size) {
+		print_mallinfo2("Test malloc 2G", &info);
+		tst_brk(TFAIL, "The member of struct mallinfo2 overflow?");
+	}
+	tst_res(TPASS, "The member of struct mallinfo2 doesn't overflow");
+	free(buf);
+}
+
+static struct tst_test test = {
+	.test_all = test_mallinfo2,
+};
+
+#else
+TST_TEST_TCONF("system doesn't implement non-POSIX mallinfo2()");
+#endif
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [LTP] [PATCH] syscalls/mallinfo201: Add a basic test for mallinfo2 when setting 2G size
  2021-05-07  7:30 [LTP] [PATCH] syscalls/mallinfo201: Add a basic test for mallinfo2 when setting 2G size Yang Xu
@ 2021-05-25 10:56 ` Petr Vorel
  2021-06-03  8:49   ` xuyang2018.jy
  2021-06-03  9:44   ` [LTP] [PATCH v2] syscalls/mallinfo2_01: " Yang Xu
  0 siblings, 2 replies; 11+ messages in thread
From: Petr Vorel @ 2021-05-25 10:56 UTC (permalink / raw)
  To: ltp

Hi Xu,

> Since these members of mallinfo struct use int data type, it will overflow
> when allocating 2G size. mallinfo() is deprecated and we should use mallinfo2()
> in the future. So we test mallinfo2 whether works well.

> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  configure.ac                                  |  1 +
>  runtest/syscalls                              |  2 +
>  .../syscalls/mallinfo/mallinfo_common.h       | 18 +++++++
>  .../kernel/syscalls/mallinfo2/.gitignore      |  1 +
>  testcases/kernel/syscalls/mallinfo2/Makefile  |  8 ++++
>  .../kernel/syscalls/mallinfo2/mallinfo201.c   | 47 +++++++++++++++++++
nit: Maybe rename test: mallinfo201.c => mallinfo2_01.c ?
Now it looks like mallinfo() 201st test.

>  6 files changed, 77 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/mallinfo2/.gitignore
>  create mode 100644 testcases/kernel/syscalls/mallinfo2/Makefile
>  create mode 100644 testcases/kernel/syscalls/mallinfo2/mallinfo201.c

...
> diff --git a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
> index a00cc7a64..e7737b270 100644
> --- a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
> +++ b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
> @@ -28,4 +28,22 @@ static inline void print_mallinfo(const char *msg, struct mallinfo *m)
>  }
>  #endif

> +#ifdef HAVE_MALLINFO2
> +static inline void print_mallinfo2(const char *msg, struct mallinfo2 *m)
> +{
> +	tst_res(TINFO, "%s...", msg);
> +#define P2(f) tst_res(TINFO, "%s: %ld", #f, m->f)
> +	P2(arena);
> +	P2(ordblks);
> +	P2(smblks);
> +	P2(hblks);
> +	P2(hblkhd);
> +	P2(usmblks);
> +	P2(fsmblks);
> +	P2(uordblks);
> +	P2(fordblks);
> +	P2(keepcost);
> +}
> +#endif

BTW did you copy this from glibc? (and previously in 7eb7a97a1)?
Code is similar as that one in malloc/tst-mallinfo2.c in glibc,
which you modified.

I guess we should also add glibc copyright to mallinfo_common.h
Copyright (C) 2020 Free Software Foundation, Inc.
(+ probably keep our)

...
> diff --git a/testcases/kernel/syscalls/mallinfo2/Makefile b/testcases/kernel/syscalls/mallinfo2/Makefile
> new file mode 100644
> index 000000000..044619fb8
> --- /dev/null
> +++ b/testcases/kernel/syscalls/mallinfo2/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) International Business Machines  Corp., 2001
Here should be your or LTP copyright with 2021.

> +
> +top_srcdir		?= ../../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
CFLAGS += -I../mallinfo
(see description below)

> +
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
...
> --- /dev/null
> +++ b/testcases/kernel/syscalls/mallinfo2/mallinfo201.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
> + */
> +
> +/*\
> + * [DESCRIPTION]
> + *
> + * Basic mallinfo2() test. Test the member of struct mallinfo2
> + * whether overflow when setting 2G size. mallinfo() is deprecated
> + * since the type used for the fields is too small. We should use
> + * mallinfo2() and it was added since glibc 2.33.
> + */

I'd format it a bit and remove info which is not related to the test and people
can find in man page.

/*\
 * [DESCRIPTION]
 *
 * Basic mallinfo2() test.
 *
 * Test members of struct mallinfo2() whether overflow when setting 2G size.
 */

> +#include "../mallinfo/mallinfo_common.h"
Please use
#include "mallinfo_common.h"

and add include path to to Makefile
CFLAGS += -I../mallinfo

> +#include "tst_safe_macros.h"
> +
> +#ifdef HAVE_MALLINFO2
> +
> +void test_mallinfo2(void)
> +{
> +	struct mallinfo2 info;
> +	char *buf;
> +	size_t size = 2UL * 1024UL * 1024UL * 1024UL;
> +
> +	buf = malloc(size);
> +	if (!buf) {
> +		tst_res(TCONF, "Current system can not malloc 2G space, skip it");
> +		return;
> +	}
BTW sometimes I wonder if we should have something like SAFE_*() functions which
would result with TCONF, not with TBROK.

> +	info = mallinfo2();
> +	if (info.hblkhd < size) {
> +		print_mallinfo2("Test malloc 2G", &info);
> +		tst_brk(TFAIL, "The member of struct mallinfo2 overflow?");
> +	}
> +	tst_res(TPASS, "The member of struct mallinfo2 doesn't overflow");
> +	free(buf);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = test_mallinfo2,
> +};
> +
> +#else
> +TST_TEST_TCONF("system doesn't implement non-POSIX mallinfo2()");
> +#endif

The rest LGTM.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH] syscalls/mallinfo201: Add a basic test for mallinfo2 when setting 2G size
  2021-05-25 10:56 ` Petr Vorel
@ 2021-06-03  8:49   ` xuyang2018.jy
  2021-06-03  9:44   ` [LTP] [PATCH v2] syscalls/mallinfo2_01: " Yang Xu
  1 sibling, 0 replies; 11+ messages in thread
From: xuyang2018.jy @ 2021-06-03  8:49 UTC (permalink / raw)
  To: ltp

Hi Petr
Sorry for late reply. I am busy recently.
> Hi Xu,
>
>> Since these members of mallinfo struct use int data type, it will overflow
>> when allocating 2G size. mallinfo() is deprecated and we should use mallinfo2()
>> in the future. So we test mallinfo2 whether works well.
>
>> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
>> ---
>>   configure.ac                                  |  1 +
>>   runtest/syscalls                              |  2 +
>>   .../syscalls/mallinfo/mallinfo_common.h       | 18 +++++++
>>   .../kernel/syscalls/mallinfo2/.gitignore      |  1 +
>>   testcases/kernel/syscalls/mallinfo2/Makefile  |  8 ++++
>>   .../kernel/syscalls/mallinfo2/mallinfo201.c   | 47 +++++++++++++++++++
> nit: Maybe rename test: mallinfo201.c =>  mallinfo2_01.c ?
> Now it looks like mallinfo() 201st test.
Agree.

>>   6 files changed, 77 insertions(+)
>>   create mode 100644 testcases/kernel/syscalls/mallinfo2/.gitignore
>>   create mode 100644 testcases/kernel/syscalls/mallinfo2/Makefile
>>   create mode 100644 testcases/kernel/syscalls/mallinfo2/mallinfo201.c
>
> ...
>> diff --git a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
>> index a00cc7a64..e7737b270 100644
>> --- a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
>> +++ b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
>> @@ -28,4 +28,22 @@ static inline void print_mallinfo(const char *msg, struct mallinfo *m)
>>   }
>>   #endif
>
>> +#ifdef HAVE_MALLINFO2
>> +static inline void print_mallinfo2(const char *msg, struct mallinfo2 *m)
>> +{
>> +	tst_res(TINFO, "%s...", msg);
>> +#define P2(f) tst_res(TINFO, "%s: %ld", #f, m->f)
>> +	P2(arena);
>> +	P2(ordblks);
>> +	P2(smblks);
>> +	P2(hblks);
>> +	P2(hblkhd);
>> +	P2(usmblks);
>> +	P2(fsmblks);
>> +	P2(uordblks);
>> +	P2(fordblks);
>> +	P2(keepcost);
>> +}
>> +#endif
>
> BTW did you copy this from glibc? (and previously in 7eb7a97a1)?
Yes.

> Code is similar as that one in malloc/tst-mallinfo2.c in glibc,
> which you modified.
>
> I guess we should also add glibc copyright to mallinfo_common.h
> Copyright (C) 2020 Free Software Foundation, Inc.
> (+ probably keep our)
I will add glibc copyright.
>
> ...
>> diff --git a/testcases/kernel/syscalls/mallinfo2/Makefile b/testcases/kernel/syscalls/mallinfo2/Makefile
>> new file mode 100644
>> index 000000000..044619fb8
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/mallinfo2/Makefile
>> @@ -0,0 +1,8 @@
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +# Copyright (c) International Business Machines  Corp., 2001
> Here should be your or LTP copyright with 2021.
>
OK.
>> +
>> +top_srcdir		?= ../../../..
>> +
>> +include $(top_srcdir)/include/mk/testcases.mk
> CFLAGS += -I../mallinfo
> (see description below)
>
>> +
>> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> ...
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/mallinfo2/mallinfo201.c
>> @@ -0,0 +1,47 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved.
>> + * Author: Yang Xu<xuyang2018.jy@fujitsu.com>
>> + */
>> +
>> +/*\
>> + * [DESCRIPTION]
>> + *
>> + * Basic mallinfo2() test. Test the member of struct mallinfo2
>> + * whether overflow when setting 2G size. mallinfo() is deprecated
>> + * since the type used for the fields is too small. We should use
>> + * mallinfo2() and it was added since glibc 2.33.
>> + */
>
> I'd format it a bit and remove info which is not related to the test and people
> can find in man page.
>
> /*\
>   * [DESCRIPTION]
>   *
>   * Basic mallinfo2() test.
>   *
>   * Test members of struct mallinfo2() whether overflow when setting 2G size.
>   */
>
Looks well.
>> +#include "../mallinfo/mallinfo_common.h"
> Please use
> #include "mallinfo_common.h"
>
> and add include path to to Makefile
> CFLAGS += -I../mallinfo
>
OK.
>> +#include "tst_safe_macros.h"
>> +
>> +#ifdef HAVE_MALLINFO2
>> +
>> +void test_mallinfo2(void)
>> +{
>> +	struct mallinfo2 info;
>> +	char *buf;
>> +	size_t size = 2UL * 1024UL * 1024UL * 1024UL;
>> +
>> +	buf = malloc(size);
>> +	if (!buf) {
>> +		tst_res(TCONF, "Current system can not malloc 2G space, skip it");
>> +		return;
>> +	}
> BTW sometimes I wonder if we should have something like SAFE_*() functions which
> would result with TCONF, not with TBROK.
IMO, I don't want to add a flag in SAFE_* functions to control return 
TCONF or TBORK because it is clear enough now. Other maintainers may 
have different option. But Now, I want to keep it.
>
>> +	info = mallinfo2();
>> +	if (info.hblkhd<  size) {
>> +		print_mallinfo2("Test malloc 2G",&info);
>> +		tst_brk(TFAIL, "The member of struct mallinfo2 overflow?");
>> +	}
>> +	tst_res(TPASS, "The member of struct mallinfo2 doesn't overflow");
>> +	free(buf);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.test_all = test_mallinfo2,
>> +};
>> +
>> +#else
>> +TST_TEST_TCONF("system doesn't implement non-POSIX mallinfo2()");
>> +#endif
>
> The rest LGTM.
Thanks for your review. I will send a v2 patch soon.
>
> Kind regards,
> Petr

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v2] syscalls/mallinfo2_01: Add a basic test for mallinfo2 when setting 2G size
  2021-05-25 10:56 ` Petr Vorel
  2021-06-03  8:49   ` xuyang2018.jy
@ 2021-06-03  9:44   ` Yang Xu
  2021-06-16  6:51     ` Petr Vorel
  2021-06-16  7:03     ` Petr Vorel
  1 sibling, 2 replies; 11+ messages in thread
From: Yang Xu @ 2021-06-03  9:44 UTC (permalink / raw)
  To: ltp

Since these members of mallinfo struct use int data type, it will overflow
when allocating 2G size. mallinfo() is deprecated and we should use mallinfo2()
in the future. So we test mallinfo2 whether works well.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 configure.ac                                  |  1 +
 runtest/syscalls                              |  2 +
 .../syscalls/mallinfo/mallinfo_common.h       | 19 ++++++++
 .../kernel/syscalls/mallinfo2/.gitignore      |  1 +
 testcases/kernel/syscalls/mallinfo2/Makefile  | 11 +++++
 .../kernel/syscalls/mallinfo2/mallinfo2_01.c  | 46 +++++++++++++++++++
 6 files changed, 80 insertions(+)
 create mode 100644 testcases/kernel/syscalls/mallinfo2/.gitignore
 create mode 100644 testcases/kernel/syscalls/mallinfo2/Makefile
 create mode 100644 testcases/kernel/syscalls/mallinfo2/mallinfo2_01.c

diff --git a/configure.ac b/configure.ac
index 136d82d09..eb675b367 100644
--- a/configure.ac
+++ b/configure.ac
@@ -102,6 +102,7 @@ AC_CHECK_FUNCS_ONCE([ \
     io_uring_enter \
     kcmp \
     mallinfo \
+    mallinfo2 \
     mallopt \
     mkdirat \
     mknodat \
diff --git a/runtest/syscalls b/runtest/syscalls
index 9df181b76..11bb58a83 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -692,6 +692,8 @@ lstat02_64 lstat02_64
 
 mallinfo02 mallinfo02
 
+mallinfo2_01 mallinfo2_01
+
 mallopt01 mallopt01
 
 mbind01 mbind01
diff --git a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
index a00cc7a64..d64a9344b 100644
--- a/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
+++ b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
+ * Copyright (C) 2020 Free Software Foundation, Inc.
  * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved.
  * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
  */
@@ -28,4 +29,22 @@ static inline void print_mallinfo(const char *msg, struct mallinfo *m)
 }
 #endif
 
+#ifdef HAVE_MALLINFO2
+static inline void print_mallinfo2(const char *msg, struct mallinfo2 *m)
+{
+	tst_res(TINFO, "%s...", msg);
+#define P2(f) tst_res(TINFO, "%s: %ld", #f, m->f)
+	P2(arena);
+	P2(ordblks);
+	P2(smblks);
+	P2(hblks);
+	P2(hblkhd);
+	P2(usmblks);
+	P2(fsmblks);
+	P2(uordblks);
+	P2(fordblks);
+	P2(keepcost);
+}
+#endif
+
 #endif
diff --git a/testcases/kernel/syscalls/mallinfo2/.gitignore b/testcases/kernel/syscalls/mallinfo2/.gitignore
new file mode 100644
index 000000000..349222c14
--- /dev/null
+++ b/testcases/kernel/syscalls/mallinfo2/.gitignore
@@ -0,0 +1 @@
+/mallinfo2_01
diff --git a/testcases/kernel/syscalls/mallinfo2/Makefile b/testcases/kernel/syscalls/mallinfo2/Makefile
new file mode 100644
index 000000000..b3c7d9552
--- /dev/null
+++ b/testcases/kernel/syscalls/mallinfo2/Makefile
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) International Business Machines  Corp., 2001
+# Copyright (c) 2021 FUJITSU LIMITED. All rights reserved.
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+CFLAGS += -I../mallinfo
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/mallinfo2/mallinfo2_01.c b/testcases/kernel/syscalls/mallinfo2/mallinfo2_01.c
new file mode 100644
index 000000000..9811f95c0
--- /dev/null
+++ b/testcases/kernel/syscalls/mallinfo2/mallinfo2_01.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [DESCRIPTION]
+ *
+ * Basic mallinfo2() test.
+ *
+ * Test members of struct mallinfo2() whether overflow when setting 2G size.
+ */
+
+#include "mallinfo_common.h"
+#include "tst_safe_macros.h"
+
+#ifdef HAVE_MALLINFO2
+
+void test_mallinfo2(void)
+{
+	struct mallinfo2 info;
+	char *buf;
+	size_t size = 2UL * 1024UL * 1024UL * 1024UL;
+
+	buf = malloc(size);
+	if (!buf) {
+		tst_res(TCONF, "Current system can not malloc 2G space, skip it");
+		return;
+	}
+	info = mallinfo2();
+	if (info.hblkhd < size) {
+		print_mallinfo2("Test malloc 2G", &info);
+		tst_brk(TFAIL, "The member of struct mallinfo2 overflow?");
+	}
+	tst_res(TPASS, "The member of struct mallinfo2 doesn't overflow");
+	free(buf);
+}
+
+static struct tst_test test = {
+	.test_all = test_mallinfo2,
+};
+
+#else
+TST_TEST_TCONF("system doesn't implement non-POSIX mallinfo2()");
+#endif
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v2] syscalls/mallinfo2_01: Add a basic test for mallinfo2 when setting 2G size
  2021-06-03  9:44   ` [LTP] [PATCH v2] syscalls/mallinfo2_01: " Yang Xu
@ 2021-06-16  6:51     ` Petr Vorel
  2021-06-16  7:00       ` xuyang2018.jy
  2021-06-16  7:03     ` Petr Vorel
  1 sibling, 1 reply; 11+ messages in thread
From: Petr Vorel @ 2021-06-16  6:51 UTC (permalink / raw)
  To: ltp

Hi Xu,

...
> +++ b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
> @@ -1,5 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /*
> + * Copyright (C) 2020 Free Software Foundation, Inc.
>   * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved.
>   * Author: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
>   */
> @@ -28,4 +29,22 @@ static inline void print_mallinfo(const char *msg, struct mallinfo *m)
>  }
>  #endif

> +#ifdef HAVE_MALLINFO2
> +static inline void print_mallinfo2(const char *msg, struct mallinfo2 *m)
> +{
> +	tst_res(TINFO, "%s...", msg);
nit: I'd remove "..." and add extra space before #define (readability)
And also change it on previously added print_mallinfo().

> +#define P2(f) tst_res(TINFO, "%s: %ld", #f, m->f)
> +	P2(arena);
> +	P2(ordblks);
> +	P2(smblks);
> +	P2(hblks);
> +	P2(hblkhd);
> +	P2(usmblks);
> +	P2(fsmblks);
> +	P2(uordblks);
> +	P2(fordblks);
> +	P2(keepcost);
> +}
> +#endif

...
> +++ b/testcases/kernel/syscalls/mallinfo2/mallinfo2_01.c
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
> + */
> +
> +/*\
> + * [DESCRIPTION]
> + *
> + * Basic mallinfo2() test.
> + *
> + * Test members of struct mallinfo2() whether overflow when setting 2G size.
* Test hblkhd member of struct mallinfo2 whether overflow when setting 2G size.

> + */
> +
> +#include "mallinfo_common.h"
> +#include "tst_safe_macros.h"
> +
> +#ifdef HAVE_MALLINFO2
> +
> +void test_mallinfo2(void)
> +{
> +	struct mallinfo2 info;
> +	char *buf;
> +	size_t size = 2UL * 1024UL * 1024UL * 1024UL;
> +
> +	buf = malloc(size);
> +	if (!buf) {
> +		tst_res(TCONF, "Current system can not malloc 2G space, skip it");
> +		return;
> +	}
Here could be just:
	if (!buf)
		tst_brk(TCONF, "Current system can not malloc 2G space, skip it");

> +	info = mallinfo2();
> +	if (info.hblkhd < size) {
> +		print_mallinfo2("Test malloc 2G", &info);
> +		tst_brk(TFAIL, "The member of struct mallinfo2 overflow?");
You don't free buf here.

Maybe something like:

	if (info.hblkhd < size) {
		print_mallinfo2("Test malloc 2G", &info);
		tst_res(TFAIL, "hblkhd member of struct mallinfo2 overflow?");
	} else {
		tst_res(TPASS, "hblkhd member of struct mallinfo2 doesn't overflow");
	}

	free(buf);

If you're ok with it, no need to repost, I'll change it before merge.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v2] syscalls/mallinfo2_01: Add a basic test for mallinfo2 when setting 2G size
  2021-06-16  6:51     ` Petr Vorel
@ 2021-06-16  7:00       ` xuyang2018.jy
  0 siblings, 0 replies; 11+ messages in thread
From: xuyang2018.jy @ 2021-06-16  7:00 UTC (permalink / raw)
  To: ltp

Hi Petr
> Hi Xu,
>
> ...
>> +++ b/testcases/kernel/syscalls/mallinfo/mallinfo_common.h
>> @@ -1,5 +1,6 @@
>>   // SPDX-License-Identifier: GPL-2.0-or-later
>>   /*
>> + * Copyright (C) 2020 Free Software Foundation, Inc.
>>    * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved.
>>    * Author: Yang Xu<xuyang2018.jy@cn.fujitsu.com>
>>    */
>> @@ -28,4 +29,22 @@ static inline void print_mallinfo(const char *msg, struct mallinfo *m)
>>   }
>>   #endif
>
>> +#ifdef HAVE_MALLINFO2
>> +static inline void print_mallinfo2(const char *msg, struct mallinfo2 *m)
>> +{
>> +	tst_res(TINFO, "%s...", msg);
> nit: I'd remove "..." and add extra space before #define (readability)
> And also change it on previously added print_mallinfo().
>
>> +#define P2(f) tst_res(TINFO, "%s: %ld", #f, m->f)
>> +	P2(arena);
>> +	P2(ordblks);
>> +	P2(smblks);
>> +	P2(hblks);
>> +	P2(hblkhd);
>> +	P2(usmblks);
>> +	P2(fsmblks);
>> +	P2(uordblks);
>> +	P2(fordblks);
>> +	P2(keepcost);
>> +}
>> +#endif
>
> ...
>> +++ b/testcases/kernel/syscalls/mallinfo2/mallinfo2_01.c
>> @@ -0,0 +1,46 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2021 FUJITSU LIMITED. All rights reserved.
>> + * Author: Yang Xu<xuyang2018.jy@fujitsu.com>
>> + */
>> +
>> +/*\
>> + * [DESCRIPTION]
>> + *
>> + * Basic mallinfo2() test.
>> + *
>> + * Test members of struct mallinfo2() whether overflow when setting 2G size.
> * Test hblkhd member of struct mallinfo2 whether overflow when setting 2G size.
>
>> + */
>> +
>> +#include "mallinfo_common.h"
>> +#include "tst_safe_macros.h"
>> +
>> +#ifdef HAVE_MALLINFO2
>> +
>> +void test_mallinfo2(void)
>> +{
>> +	struct mallinfo2 info;
>> +	char *buf;
>> +	size_t size = 2UL * 1024UL * 1024UL * 1024UL;
>> +
>> +	buf = malloc(size);
>> +	if (!buf) {
>> +		tst_res(TCONF, "Current system can not malloc 2G space, skip it");
>> +		return;
>> +	}
> Here could be just:
> 	if (!buf)
> 		tst_brk(TCONF, "Current system can not malloc 2G space, skip it");
>
>> +	info = mallinfo2();
>> +	if (info.hblkhd<  size) {
>> +		print_mallinfo2("Test malloc 2G",&info);
>> +		tst_brk(TFAIL, "The member of struct mallinfo2 overflow?");
> You don't free buf here.
>
> Maybe something like:
>
> 	if (info.hblkhd<  size) {
> 		print_mallinfo2("Test malloc 2G",&info);
> 		tst_res(TFAIL, "hblkhd member of struct mallinfo2 overflow?");
> 	} else {
> 		tst_res(TPASS, "hblkhd member of struct mallinfo2 doesn't overflow");
> 	}
>
> 	free(buf);
>
> If you're ok with it, no need to repost, I'll change it before merge.
I am ok with these changes. Thanks for your review.

Best Regards
Yang Xu
>
> Kind regards,
> Petr

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v2] syscalls/mallinfo2_01: Add a basic test for mallinfo2 when setting 2G size
  2021-06-03  9:44   ` [LTP] [PATCH v2] syscalls/mallinfo2_01: " Yang Xu
  2021-06-16  6:51     ` Petr Vorel
@ 2021-06-16  7:03     ` Petr Vorel
  2021-06-16  7:22       ` xuyang2018.jy
  2021-06-16  9:16       ` Joerg Vehlow
  1 sibling, 2 replies; 11+ messages in thread
From: Petr Vorel @ 2021-06-16  7:03 UTC (permalink / raw)
  To: ltp

Hi Xu,

> +++ b/testcases/kernel/syscalls/mallinfo2/Makefile
...
> +
> +top_srcdir		?= ../../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +
> +CFLAGS += -I../mallinfo
Also this needs to be
CFLAGS += -I$(abs_srcdir)/../mallinfo

otherwise out of tree builds fail:
https://github.com/pevik/ltp/actions/runs/941820122

BTW Cyril also suggested recently to drop out of tree support, because build
system dependencies are broken and fixing it would be much easier when
supporting only in tree build.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v2] syscalls/mallinfo2_01: Add a basic test for mallinfo2 when setting 2G size
  2021-06-16  7:03     ` Petr Vorel
@ 2021-06-16  7:22       ` xuyang2018.jy
  2021-06-16  8:51         ` Petr Vorel
  2021-06-16  9:16       ` Joerg Vehlow
  1 sibling, 1 reply; 11+ messages in thread
From: xuyang2018.jy @ 2021-06-16  7:22 UTC (permalink / raw)
  To: ltp

Hi Petr
> Hi Xu,
>
>> +++ b/testcases/kernel/syscalls/mallinfo2/Makefile
> ...
>> +
>> +top_srcdir		?= ../../../..
>> +
>> +include $(top_srcdir)/include/mk/testcases.mk
>> +
>> +CFLAGS += -I../mallinfo
> Also this needs to be
> CFLAGS += -I$(abs_srcdir)/../mallinfo
>
You are right. It will fail when out of tree.
> otherwise out of tree builds fail:
> https://github.com/pevik/ltp/actions/runs/941820122
>
> BTW Cyril also suggested recently to drop out of tree support, because build
> system dependencies are broken and fixing it would be much easier when
> supporting only in tree build.
On my test envrionment, I never use out of tree build. So I am fine with 
removing it.
>
> Kind regards,
> Petr

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v2] syscalls/mallinfo2_01: Add a basic test for mallinfo2 when setting 2G size
  2021-06-16  7:22       ` xuyang2018.jy
@ 2021-06-16  8:51         ` Petr Vorel
  0 siblings, 0 replies; 11+ messages in thread
From: Petr Vorel @ 2021-06-16  8:51 UTC (permalink / raw)
  To: ltp

Hi Xu,

Merged with all my suggested changes + slightly adjusted docparse description
and commit message.

Thanks a lot!

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v2] syscalls/mallinfo2_01: Add a basic test for mallinfo2 when setting 2G size
  2021-06-16  7:03     ` Petr Vorel
  2021-06-16  7:22       ` xuyang2018.jy
@ 2021-06-16  9:16       ` Joerg Vehlow
  2021-06-16  9:28         ` Cyril Hrubis
  1 sibling, 1 reply; 11+ messages in thread
From: Joerg Vehlow @ 2021-06-16  9:16 UTC (permalink / raw)
  To: ltp

Hi,

On 6/16/2021 9:03 AM, Petr Vorel wrote:
>
> BTW Cyril also suggested recently to drop out of tree support, because build
> system dependencies are broken and fixing it would be much easier when
> supporting only in tree build.
Really? I got the feeling out-of-tree builds are becoming the new normal 
and even complex pieces of software like the kernel, glibc and gcc are 
able to be built out-of-tree or even enforce it.
Maybe it is time to abandon autoconf/automake and switch to a better 
build configuration systems like cmake instead of abandoning out-of-tree 
builds.

J?rg

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [LTP] [PATCH v2] syscalls/mallinfo2_01: Add a basic test for mallinfo2 when setting 2G size
  2021-06-16  9:16       ` Joerg Vehlow
@ 2021-06-16  9:28         ` Cyril Hrubis
  0 siblings, 0 replies; 11+ messages in thread
From: Cyril Hrubis @ 2021-06-16  9:28 UTC (permalink / raw)
  To: ltp

Hi!
> > BTW Cyril also suggested recently to drop out of tree support, because build
> > system dependencies are broken and fixing it would be much easier when
> > supporting only in tree build.
> Really? I got the feeling out-of-tree builds are becoming the new normal 
> and even complex pieces of software like the kernel, glibc and gcc are 
> able to be built out-of-tree or even enforce it.

The question here really if support of out-of-tree build is worth the
trouble. The problem is that the complexity it adds to the build system
is quite high. The way it's implemented in LTP the build system has to
work with absolute paths and you have to make sure you have the right
path to a file all the time and people are often confused by this. Also
it crashes and burns with cryptic error messages if you happen to have
characters interpreted by make in a directory name anywhere from the LTP
dir to the filesystems root (try adding : to a directory name for
instance). We used to have out-of-tree build broken for years, before we
set up a CI, and nobody complained. So considering all of this it may be
for the better to just get rid of it.

I'm not against out-of-tree build in principle but given that the way
it's implemented in LTP is subtly broken by design and that there does
not seem to be real users it does not make that much sense to keep it
maintained and spend any effort to keep it afloat.

> Maybe it is time to abandon autoconf/automake and switch to a better 
> build configuration systems like cmake instead of abandoning out-of-tree 
> builds.

I'm not againts this, the main problem is that this will take a
significant effort my guess is at least month to implement and and even
more to iron out all the corner cases. Also the choosen build systems
has to be generally available even on 10 years old system, but I guess
that cmake should classify easily here.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-06-16  9:28 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-07  7:30 [LTP] [PATCH] syscalls/mallinfo201: Add a basic test for mallinfo2 when setting 2G size Yang Xu
2021-05-25 10:56 ` Petr Vorel
2021-06-03  8:49   ` xuyang2018.jy
2021-06-03  9:44   ` [LTP] [PATCH v2] syscalls/mallinfo2_01: " Yang Xu
2021-06-16  6:51     ` Petr Vorel
2021-06-16  7:00       ` xuyang2018.jy
2021-06-16  7:03     ` Petr Vorel
2021-06-16  7:22       ` xuyang2018.jy
2021-06-16  8:51         ` Petr Vorel
2021-06-16  9:16       ` Joerg Vehlow
2021-06-16  9:28         ` Cyril Hrubis

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.