* [LTP] [PATCH] syscalls/memfd_create: test memfd flags separately
@ 2017-05-30 7:59 Jakub Racek
2017-05-31 10:56 ` Cyril Hrubis
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Racek @ 2017-05-30 7:59 UTC (permalink / raw)
To: ltp
This patch fixes the test on some kernels where memfd_create doesn't
have have sealing support. Calls to memfd_create are done so that we
know what flags are available. Depending on that, some tests are skipped
instead of incorrectly reporting breakage.
Signed-off-by: Jakub Racek <jracek@redhat.com>
---
.../kernel/syscalls/memfd_create/memfd_create01.c | 16 +++++++-
.../kernel/syscalls/memfd_create/memfd_create02.c | 25 +++++++-----
.../syscalls/memfd_create/memfd_create_common.c | 44 +++++++++++++++++++---
.../syscalls/memfd_create/memfd_create_common.h | 12 ++++--
4 files changed, 78 insertions(+), 19 deletions(-)
diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create01.c b/testcases/kernel/syscalls/memfd_create/memfd_create01.c
index 54c817b..c5e76f2 100644
--- a/testcases/kernel/syscalls/memfd_create/memfd_create01.c
+++ b/testcases/kernel/syscalls/memfd_create/memfd_create01.c
@@ -22,6 +22,7 @@
#define _GNU_SOURCE
+#include <errno.h>
#include "tst_test.h"
#include "memfd_create_common.h"
@@ -259,7 +260,20 @@ static void verify_memfd_create(unsigned int n)
static void setup(void)
{
- ASSERT_HAVE_MEMFD_CREATE();
+
+ int available_flags;
+
+ /*
+ * For now, all tests in this file require MFD_ALLOW_SEALING flag
+ * to be implemented, even though that flag isn't always set when
+ * memfd is created. So don't check anything else and TCONF right away
+ * is this flag is missing.
+ */
+ available_flags = GET_MFD_ALL_AVAILABLE_FLAGS();
+ if ((available_flags & MFD_ALLOW_SEALING) != MFD_ALLOW_SEALING) {
+ tst_brk(TCONF | TTERRNO,
+ "memfd_create(%u) not implemented", MFD_ALLOW_SEALING);
+ }
}
static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create02.c b/testcases/kernel/syscalls/memfd_create/memfd_create02.c
index d65e496..0e616d7 100644
--- a/testcases/kernel/syscalls/memfd_create/memfd_create02.c
+++ b/testcases/kernel/syscalls/memfd_create/memfd_create02.c
@@ -31,6 +31,8 @@
static char buf[2048];
static char term_buf[2048];
+static int available_flags;
+
static const struct tcase {
char *descr;
char *memfd_name;
@@ -62,7 +64,8 @@ static const struct tcase {
static void setup(void)
{
- ASSERT_HAVE_MEMFD_CREATE();
+
+ available_flags = GET_MFD_ALL_AVAILABLE_FLAGS();
memset(buf, 0xff, sizeof(buf));
@@ -76,14 +79,18 @@ static void verify_memfd_create_errno(unsigned int n)
tc = &tcases[n];
- TEST(sys_memfd_create(tc->memfd_name, tc->flags));
- if (TEST_ERRNO != tc->memfd_create_exp_err)
- tst_brk(TFAIL, "test '%s'", tc->descr);
- else
- tst_res(TPASS, "test '%s'", tc->descr);
-
- if (TEST_RETURN > 0)
- SAFE_CLOSE(TEST_RETURN);
+ if ((available_flags & tc->flags) == tc->flags) {
+ TEST(sys_memfd_create(tc->memfd_name, tc->flags));
+ if (tc->memfd_create_exp_err != TEST_ERRNO)
+ tst_brk(TFAIL, "test '%s'", tc->descr);
+ else
+ tst_res(TPASS, "test '%s'", tc->descr);
+
+ if (TEST_RETURN > 0)
+ SAFE_CLOSE(TEST_RETURN);
+ } else
+ tst_res(TCONF, "test '%s' skipped, flag not implemented",
+ tc->descr);
}
static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create_common.c b/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
index 4cf3bd5..f87c858 100644
--- a/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
+++ b/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
@@ -103,20 +103,52 @@ void check_ftruncate_fail(const char *filename, const int lineno,
"ftruncate(%d, %ld) failed as expected", fd, length);
}
-void assert_have_memfd_create(const char *filename, const int lineno)
+int get_mfd_all_available_flags(const char *filename, const int lineno)
{
- TEST(sys_memfd_create("dummy_call", 0));
+ unsigned int i;
+ int flag;
+ int flags2test[] = {0, MFD_CLOEXEC, MFD_ALLOW_SEALING};
+ int flags_available = 0;
+ int any_available = 0;
+
+ for (i = 0; i < ARRAY_SIZE(flags2test); i++) {
+ flag = flags2test[i];
+
+ TEST(check_mfd_available_with_flags(flag));
+ if (TEST_RETURN == MFD_FLAG_NOT_IMPLEMENTED) {
+ tst_res_(filename, lineno, TINFO | TTERRNO,
+ "memfd_create(%u) not implemented", flag);
+ } else if (TEST_RETURN == MFD_FAILED) {
+ tst_res_(filename, lineno, TINFO | TTERRNO,
+ "memfd_create(%u) failed", flag);
+ } else {
+ flags_available |= flag;
+ any_available = 1;
+ }
+ }
+
+ if (!any_available) {
+ tst_brk_(filename, lineno, TCONF,
+ "memfd_create() not implemented@all");
+ }
+
+ return flags_available;
+}
+
+int check_mfd_available_with_flags(unsigned int flags)
+{
+ TEST(sys_memfd_create("dummy_call", flags));
if (TEST_RETURN < 0) {
if (TEST_ERRNO == EINVAL) {
- tst_brk_(filename, lineno, TCONF | TTERRNO,
- "memfd_create() not implemented");
+ return MFD_FLAG_NOT_IMPLEMENTED;
}
- tst_brk_(filename, lineno, TBROK | TTERRNO,
- "memfd_create() failed");
+ return MFD_FAILED;
}
SAFE_CLOSE(TEST_RETURN);
+
+ return MFD_AVAILABLE;
}
int check_mfd_new(const char *filename, const int lineno,
diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create_common.h b/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
index 6329ac3..c90d1f7 100644
--- a/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
+++ b/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
@@ -22,8 +22,12 @@
#define MFD_DEF_SIZE 8192
-#define ASSERT_HAVE_MEMFD_CREATE() \
- assert_have_memfd_create(__FILE__, __LINE__)
+#define MFD_AVAILABLE 0
+#define MFD_FAILED 1
+#define MFD_FLAG_NOT_IMPLEMENTED 2
+
+#define GET_MFD_ALL_AVAILABLE_FLAGS() \
+ get_mfd_all_available_flags(__FILE__, __LINE__)
#define CHECK_MFD_NEW(name, sz, flags) \
check_mfd_new(__FILE__, __LINE__, (name), (sz), (flags))
@@ -89,7 +93,9 @@
#define CHECK_MFD_NON_GROWABLE_BY_WRITE(fd) \
check_mfd_non_growable_by_write(__FILE__, __LINE__, (fd))
-void assert_have_memfd_create(const char *filename, const int lineno);
+int check_mfd_available_with_flags(unsigned int flags);
+int get_mfd_all_available_flags(const char *filename, const int lineno);
+
int sys_memfd_create(const char *name, unsigned int flags);
int check_fallocate(const char *filename, const int lineno, int fd,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [LTP] [PATCH] syscalls/memfd_create: test memfd flags separately
2017-05-30 7:59 [LTP] [PATCH] syscalls/memfd_create: test memfd flags separately Jakub Racek
@ 2017-05-31 10:56 ` Cyril Hrubis
2017-06-01 11:33 ` Jakub =?unknown-8bit?q?Ra=C4=8Dek?=
0 siblings, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2017-05-31 10:56 UTC (permalink / raw)
To: ltp
Hi!
> This patch fixes the test on some kernels where memfd_create doesn't
> have have sealing support. Calls to memfd_create are done so that we
> know what flags are available. Depending on that, some tests are skipped
> instead of incorrectly reporting breakage.
>
> Signed-off-by: Jakub Racek <jracek@redhat.com>
> ---
> .../kernel/syscalls/memfd_create/memfd_create01.c | 16 +++++++-
> .../kernel/syscalls/memfd_create/memfd_create02.c | 25 +++++++-----
> .../syscalls/memfd_create/memfd_create_common.c | 44 +++++++++++++++++++---
> .../syscalls/memfd_create/memfd_create_common.h | 12 ++++--
> 4 files changed, 78 insertions(+), 19 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create01.c b/testcases/kernel/syscalls/memfd_create/memfd_create01.c
> index 54c817b..c5e76f2 100644
> --- a/testcases/kernel/syscalls/memfd_create/memfd_create01.c
> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create01.c
> @@ -22,6 +22,7 @@
>
> #define _GNU_SOURCE
>
> +#include <errno.h>
> #include "tst_test.h"
> #include "memfd_create_common.h"
>
> @@ -259,7 +260,20 @@ static void verify_memfd_create(unsigned int n)
>
> static void setup(void)
> {
> - ASSERT_HAVE_MEMFD_CREATE();
> +
> + int available_flags;
> +
> + /*
> + * For now, all tests in this file require MFD_ALLOW_SEALING flag
> + * to be implemented, even though that flag isn't always set when
> + * memfd is created. So don't check anything else and TCONF right away
> + * is this flag is missing.
> + */
> + available_flags = GET_MFD_ALL_AVAILABLE_FLAGS();
> + if ((available_flags & MFD_ALLOW_SEALING) != MFD_ALLOW_SEALING) {
> + tst_brk(TCONF | TTERRNO,
> + "memfd_create(%u) not implemented", MFD_ALLOW_SEALING);
> + }
> }
>
> static struct tst_test test = {
> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create02.c b/testcases/kernel/syscalls/memfd_create/memfd_create02.c
> index d65e496..0e616d7 100644
> --- a/testcases/kernel/syscalls/memfd_create/memfd_create02.c
> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create02.c
> @@ -31,6 +31,8 @@
> static char buf[2048];
> static char term_buf[2048];
>
> +static int available_flags;
> +
> static const struct tcase {
> char *descr;
> char *memfd_name;
> @@ -62,7 +64,8 @@ static const struct tcase {
>
> static void setup(void)
> {
> - ASSERT_HAVE_MEMFD_CREATE();
> +
> + available_flags = GET_MFD_ALL_AVAILABLE_FLAGS();
>
> memset(buf, 0xff, sizeof(buf));
>
> @@ -76,14 +79,18 @@ static void verify_memfd_create_errno(unsigned int n)
>
> tc = &tcases[n];
>
> - TEST(sys_memfd_create(tc->memfd_name, tc->flags));
> - if (TEST_ERRNO != tc->memfd_create_exp_err)
> - tst_brk(TFAIL, "test '%s'", tc->descr);
> - else
> - tst_res(TPASS, "test '%s'", tc->descr);
> -
> - if (TEST_RETURN > 0)
> - SAFE_CLOSE(TEST_RETURN);
> + if ((available_flags & tc->flags) == tc->flags) {
> + TEST(sys_memfd_create(tc->memfd_name, tc->flags));
> + if (tc->memfd_create_exp_err != TEST_ERRNO)
> + tst_brk(TFAIL, "test '%s'", tc->descr);
> + else
> + tst_res(TPASS, "test '%s'", tc->descr);
> +
> + if (TEST_RETURN > 0)
> + SAFE_CLOSE(TEST_RETURN);
> + } else
> + tst_res(TCONF, "test '%s' skipped, flag not implemented",
> + tc->descr);
It would be a bit cleaner to check for the flags first and return from
the function if flag is not available, i.e.
if ((available_flags & tc->flags) != tc->flags) {
tst_res(TCONF, ...);
return;
}
TEST(sys_memfd_create(...));
...
> }
>
> static struct tst_test test = {
> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create_common.c b/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
> index 4cf3bd5..f87c858 100644
> --- a/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
> @@ -103,20 +103,52 @@ void check_ftruncate_fail(const char *filename, const int lineno,
> "ftruncate(%d, %ld) failed as expected", fd, length);
> }
>
> -void assert_have_memfd_create(const char *filename, const int lineno)
> +int get_mfd_all_available_flags(const char *filename, const int lineno)
> {
> - TEST(sys_memfd_create("dummy_call", 0));
> + unsigned int i;
> + int flag;
> + int flags2test[] = {0, MFD_CLOEXEC, MFD_ALLOW_SEALING};
> + int flags_available = 0;
> + int any_available = 0;
> +
> + for (i = 0; i < ARRAY_SIZE(flags2test); i++) {
> + flag = flags2test[i];
> +
> + TEST(check_mfd_available_with_flags(flag));
> + if (TEST_RETURN == MFD_FLAG_NOT_IMPLEMENTED) {
> + tst_res_(filename, lineno, TINFO | TTERRNO,
> + "memfd_create(%u) not implemented", flag);
> + } else if (TEST_RETURN == MFD_FAILED) {
> + tst_res_(filename, lineno, TINFO | TTERRNO,
> + "memfd_create(%u) failed", flag);
> + } else {
> + flags_available |= flag;
> + any_available = 1;
> + }
> + }
> +
> + if (!any_available) {
> + tst_brk_(filename, lineno, TCONF,
> + "memfd_create() not implemented at all");
> + }
Hmm, I wonder if there could be a situation where memfd_create() with
flags = 0 will fail but memfd_creat() with non-zero flags will not. I
doubt so but as the code is that will create a situaion where tests
with flags = 0 will fail as well.
So what about checking for flags=0 first, doing tst_brk(TCONF, ...) if
that is not working, then loop over the MFD_CLOEXEC and
MFD_ALLOW_SEALING flags?
> + return flags_available;
> +}
> +
> +int check_mfd_available_with_flags(unsigned int flags)
> +{
> + TEST(sys_memfd_create("dummy_call", flags));
> if (TEST_RETURN < 0) {
> if (TEST_ERRNO == EINVAL) {
> - tst_brk_(filename, lineno, TCONF | TTERRNO,
> - "memfd_create() not implemented");
> + return MFD_FLAG_NOT_IMPLEMENTED;
> }
>
> - tst_brk_(filename, lineno, TBROK | TTERRNO,
> - "memfd_create() failed");
> + return MFD_FAILED;
I would argue that if the mfd_create() has failed unexpectedly here the
right thing to do is tst_brk(TBROK | TTERRNO, ...), there is no point in
continuing the test if the syscall is failing for unknown reason, in
that case the best course of action is to die quickly with a good
description of the problem. Which also means that there is no point in
returning anything else than 0 and 1 in this function, just return 0 if
flags are not available and 1 if they are.
> }
>
> SAFE_CLOSE(TEST_RETURN);
> +
> + return MFD_AVAILABLE;
> }
>
> int check_mfd_new(const char *filename, const int lineno,
> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create_common.h b/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
> index 6329ac3..c90d1f7 100644
> --- a/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
> @@ -22,8 +22,12 @@
>
> #define MFD_DEF_SIZE 8192
>
> -#define ASSERT_HAVE_MEMFD_CREATE() \
> - assert_have_memfd_create(__FILE__, __LINE__)
> +#define MFD_AVAILABLE 0
> +#define MFD_FAILED 1
> +#define MFD_FLAG_NOT_IMPLEMENTED 2
> +
> +#define GET_MFD_ALL_AVAILABLE_FLAGS() \
> + get_mfd_all_available_flags(__FILE__, __LINE__)
>
> #define CHECK_MFD_NEW(name, sz, flags) \
> check_mfd_new(__FILE__, __LINE__, (name), (sz), (flags))
> @@ -89,7 +93,9 @@
> #define CHECK_MFD_NON_GROWABLE_BY_WRITE(fd) \
> check_mfd_non_growable_by_write(__FILE__, __LINE__, (fd))
>
> -void assert_have_memfd_create(const char *filename, const int lineno);
> +int check_mfd_available_with_flags(unsigned int flags);
> +int get_mfd_all_available_flags(const char *filename, const int lineno);
> +
> int sys_memfd_create(const char *name, unsigned int flags);
>
> int check_fallocate(const char *filename, const int lineno, int fd,
> --
> 1.8.3.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH] syscalls/memfd_create: test memfd flags separately
2017-05-31 10:56 ` Cyril Hrubis
@ 2017-06-01 11:33 ` Jakub =?unknown-8bit?q?Ra=C4=8Dek?=
0 siblings, 0 replies; 6+ messages in thread
From: Jakub =?unknown-8bit?q?Ra=C4=8Dek?= @ 2017-06-01 11:33 UTC (permalink / raw)
To: ltp
Hi,
On 05/31/2017 12:56 PM, Cyril Hrubis wrote:
> Hi!
>> This patch fixes the test on some kernels where memfd_create doesn't
>> have have sealing support. Calls to memfd_create are done so that we
>> know what flags are available. Depending on that, some tests are skipped
>> instead of incorrectly reporting breakage.
>>
>> Signed-off-by: Jakub Racek <jracek@redhat.com>
>> ---
>> .../kernel/syscalls/memfd_create/memfd_create01.c | 16 +++++++-
>> .../kernel/syscalls/memfd_create/memfd_create02.c | 25 +++++++-----
>> .../syscalls/memfd_create/memfd_create_common.c | 44 +++++++++++++++++++---
>> .../syscalls/memfd_create/memfd_create_common.h | 12 ++++--
>> 4 files changed, 78 insertions(+), 19 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create01.c b/testcases/kernel/syscalls/memfd_create/memfd_create01.c
>> index 54c817b..c5e76f2 100644
>> --- a/testcases/kernel/syscalls/memfd_create/memfd_create01.c
>> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create01.c
>> @@ -22,6 +22,7 @@
>>
>> #define _GNU_SOURCE
>>
>> +#include <errno.h>
>> #include "tst_test.h"
>> #include "memfd_create_common.h"
>>
>> @@ -259,7 +260,20 @@ static void verify_memfd_create(unsigned int n)
>>
>> static void setup(void)
>> {
>> - ASSERT_HAVE_MEMFD_CREATE();
>> +
>> + int available_flags;
>> +
>> + /*
>> + * For now, all tests in this file require MFD_ALLOW_SEALING flag
>> + * to be implemented, even though that flag isn't always set when
>> + * memfd is created. So don't check anything else and TCONF right away
>> + * is this flag is missing.
>> + */
>> + available_flags = GET_MFD_ALL_AVAILABLE_FLAGS();
>> + if ((available_flags & MFD_ALLOW_SEALING) != MFD_ALLOW_SEALING) {
>> + tst_brk(TCONF | TTERRNO,
>> + "memfd_create(%u) not implemented", MFD_ALLOW_SEALING);
>> + }
>> }
>>
>> static struct tst_test test = {
>> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create02.c b/testcases/kernel/syscalls/memfd_create/memfd_create02.c
>> index d65e496..0e616d7 100644
>> --- a/testcases/kernel/syscalls/memfd_create/memfd_create02.c
>> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create02.c
>> @@ -31,6 +31,8 @@
>> static char buf[2048];
>> static char term_buf[2048];
>>
>> +static int available_flags;
>> +
>> static const struct tcase {
>> char *descr;
>> char *memfd_name;
>> @@ -62,7 +64,8 @@ static const struct tcase {
>>
>> static void setup(void)
>> {
>> - ASSERT_HAVE_MEMFD_CREATE();
>> +
>> + available_flags = GET_MFD_ALL_AVAILABLE_FLAGS();
>>
>> memset(buf, 0xff, sizeof(buf));
>>
>> @@ -76,14 +79,18 @@ static void verify_memfd_create_errno(unsigned int n)
>>
>> tc = &tcases[n];
>>
>> - TEST(sys_memfd_create(tc->memfd_name, tc->flags));
>> - if (TEST_ERRNO != tc->memfd_create_exp_err)
>> - tst_brk(TFAIL, "test '%s'", tc->descr);
>> - else
>> - tst_res(TPASS, "test '%s'", tc->descr);
>> -
>> - if (TEST_RETURN > 0)
>> - SAFE_CLOSE(TEST_RETURN);
>> + if ((available_flags & tc->flags) == tc->flags) {
>> + TEST(sys_memfd_create(tc->memfd_name, tc->flags));
>> + if (tc->memfd_create_exp_err != TEST_ERRNO)
>> + tst_brk(TFAIL, "test '%s'", tc->descr);
>> + else
>> + tst_res(TPASS, "test '%s'", tc->descr);
>> +
>> + if (TEST_RETURN > 0)
>> + SAFE_CLOSE(TEST_RETURN);
>> + } else
>> + tst_res(TCONF, "test '%s' skipped, flag not implemented",
>> + tc->descr);
>
> It would be a bit cleaner to check for the flags first and return from
> the function if flag is not available, i.e.
>
>
> if ((available_flags & tc->flags) != tc->flags) {
> tst_res(TCONF, ...);
> return;
> }
>
> TEST(sys_memfd_create(...));
> ...
>
>> }
>>
Good catch, this actually has to be changed even more than that since
memfd_create02 tests how api responds to invalid flags.
That and all other suggestions are reflected in V2 I'm sending now.
>> static struct tst_test test = {
>> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create_common.c b/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
>> index 4cf3bd5..f87c858 100644
>> --- a/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
>> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
>> @@ -103,20 +103,52 @@ void check_ftruncate_fail(const char *filename, const int lineno,
>> "ftruncate(%d, %ld) failed as expected", fd, length);
>> }
>>
>> -void assert_have_memfd_create(const char *filename, const int lineno)
>> +int get_mfd_all_available_flags(const char *filename, const int lineno)
>> {
>> - TEST(sys_memfd_create("dummy_call", 0));
>> + unsigned int i;
>> + int flag;
>> + int flags2test[] = {0, MFD_CLOEXEC, MFD_ALLOW_SEALING};
>> + int flags_available = 0;
>> + int any_available = 0;
>> +
>> + for (i = 0; i < ARRAY_SIZE(flags2test); i++) {
>> + flag = flags2test[i];
>> +
>> + TEST(check_mfd_available_with_flags(flag));
>> + if (TEST_RETURN == MFD_FLAG_NOT_IMPLEMENTED) {
>> + tst_res_(filename, lineno, TINFO | TTERRNO,
>> + "memfd_create(%u) not implemented", flag);
>> + } else if (TEST_RETURN == MFD_FAILED) {
>> + tst_res_(filename, lineno, TINFO | TTERRNO,
>> + "memfd_create(%u) failed", flag);
>> + } else {
>> + flags_available |= flag;
>> + any_available = 1;
>> + }
>> + }
>> +
>> + if (!any_available) {
>> + tst_brk_(filename, lineno, TCONF,
>> + "memfd_create() not implemented at all");
>> + }
>
> Hmm, I wonder if there could be a situation where memfd_create() with
> flags = 0 will fail but memfd_creat() with non-zero flags will not. I
> doubt so but as the code is that will create a situaion where tests
> with flags = 0 will fail as well.
>
> So what about checking for flags=0 first, doing tst_brk(TCONF, ...) if
> that is not working, then loop over the MFD_CLOEXEC and
> MFD_ALLOW_SEALING flags?
>
>> + return flags_available;
>> +}
>> +
>> +int check_mfd_available_with_flags(unsigned int flags)
>> +{
>> + TEST(sys_memfd_create("dummy_call", flags));
>> if (TEST_RETURN < 0) {
>> if (TEST_ERRNO == EINVAL) {
>> - tst_brk_(filename, lineno, TCONF | TTERRNO,
>> - "memfd_create() not implemented");
>> + return MFD_FLAG_NOT_IMPLEMENTED;
>> }
>>
>> - tst_brk_(filename, lineno, TBROK | TTERRNO,
>> - "memfd_create() failed");
>> + return MFD_FAILED;
>
> I would argue that if the mfd_create() has failed unexpectedly here the
> right thing to do is tst_brk(TBROK | TTERRNO, ...), there is no point in
> continuing the test if the syscall is failing for unknown reason, in
> that case the best course of action is to die quickly with a good
> description of the problem. Which also means that there is no point in
> returning anything else than 0 and 1 in this function, just return 0 if
> flags are not available and 1 if they are.
>
>> }
>>
>> SAFE_CLOSE(TEST_RETURN);
>> +
>> + return MFD_AVAILABLE;
>> }
>>
>> int check_mfd_new(const char *filename, const int lineno,
>> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create_common.h b/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
>> index 6329ac3..c90d1f7 100644
>> --- a/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
>> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
>> @@ -22,8 +22,12 @@
>>
>> #define MFD_DEF_SIZE 8192
>>
>> -#define ASSERT_HAVE_MEMFD_CREATE() \
>> - assert_have_memfd_create(__FILE__, __LINE__)
>> +#define MFD_AVAILABLE 0
>> +#define MFD_FAILED 1
>> +#define MFD_FLAG_NOT_IMPLEMENTED 2
>> +
>> +#define GET_MFD_ALL_AVAILABLE_FLAGS() \
>> + get_mfd_all_available_flags(__FILE__, __LINE__)
>>
>> #define CHECK_MFD_NEW(name, sz, flags) \
>> check_mfd_new(__FILE__, __LINE__, (name), (sz), (flags))
>> @@ -89,7 +93,9 @@
>> #define CHECK_MFD_NON_GROWABLE_BY_WRITE(fd) \
>> check_mfd_non_growable_by_write(__FILE__, __LINE__, (fd))
>>
>> -void assert_have_memfd_create(const char *filename, const int lineno);
>> +int check_mfd_available_with_flags(unsigned int flags);
>> +int get_mfd_all_available_flags(const char *filename, const int lineno);
>> +
>> int sys_memfd_create(const char *name, unsigned int flags);
>>
>> int check_fallocate(const char *filename, const int lineno, int fd,
>> --
>> 1.8.3.1
>>
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp
>
Thank you.
--
Regards,
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH] syscalls/memfd_create: test memfd flags separately
@ 2017-06-01 11:34 Jakub Racek
2017-06-06 13:15 ` Cyril Hrubis
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Racek @ 2017-06-01 11:34 UTC (permalink / raw)
To: ltp
This patch fixes the test on some kernels where memfd_create doesn't
have have sealing support. Calls to memfd_create are done so that we
know what flags are available. Depending on that, some tests are skipped
instead of incorrectly reporting breakage.
Signed-off-by: Jakub Racek <jracek@redhat.com>
---
.../kernel/syscalls/memfd_create/memfd_create01.c | 16 ++++++-
.../kernel/syscalls/memfd_create/memfd_create02.c | 13 +++++-
.../syscalls/memfd_create/memfd_create_common.c | 50 +++++++++++++++++++---
.../syscalls/memfd_create/memfd_create_common.h | 19 ++++++--
4 files changed, 86 insertions(+), 12 deletions(-)
diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create01.c b/testcases/kernel/syscalls/memfd_create/memfd_create01.c
index 54c817b..c5e76f2 100644
--- a/testcases/kernel/syscalls/memfd_create/memfd_create01.c
+++ b/testcases/kernel/syscalls/memfd_create/memfd_create01.c
@@ -22,6 +22,7 @@
#define _GNU_SOURCE
+#include <errno.h>
#include "tst_test.h"
#include "memfd_create_common.h"
@@ -259,7 +260,20 @@ static void verify_memfd_create(unsigned int n)
static void setup(void)
{
- ASSERT_HAVE_MEMFD_CREATE();
+
+ int available_flags;
+
+ /*
+ * For now, all tests in this file require MFD_ALLOW_SEALING flag
+ * to be implemented, even though that flag isn't always set when
+ * memfd is created. So don't check anything else and TCONF right away
+ * is this flag is missing.
+ */
+ available_flags = GET_MFD_ALL_AVAILABLE_FLAGS();
+ if ((available_flags & MFD_ALLOW_SEALING) != MFD_ALLOW_SEALING) {
+ tst_brk(TCONF | TTERRNO,
+ "memfd_create(%u) not implemented", MFD_ALLOW_SEALING);
+ }
}
static struct tst_test test = {
diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create02.c b/testcases/kernel/syscalls/memfd_create/memfd_create02.c
index d65e496..bc01e44 100644
--- a/testcases/kernel/syscalls/memfd_create/memfd_create02.c
+++ b/testcases/kernel/syscalls/memfd_create/memfd_create02.c
@@ -31,6 +31,8 @@
static char buf[2048];
static char term_buf[2048];
+static int available_flags;
+
static const struct tcase {
char *descr;
char *memfd_name;
@@ -62,7 +64,8 @@ static const struct tcase {
static void setup(void)
{
- ASSERT_HAVE_MEMFD_CREATE();
+
+ available_flags = GET_MFD_ALL_AVAILABLE_FLAGS();
memset(buf, 0xff, sizeof(buf));
@@ -73,8 +76,16 @@ static void setup(void)
static void verify_memfd_create_errno(unsigned int n)
{
const struct tcase *tc;
+ int needed_flags;
tc = &tcases[n];
+ needed_flags = tc->flags & FLAGS_ALL_MASK;
+
+ if ((available_flags & needed_flags) != needed_flags) {
+ tst_res(TCONF, "test '%s' skipped, flag not implemented",
+ tc->descr);
+ return;
+ }
TEST(sys_memfd_create(tc->memfd_name, tc->flags));
if (TEST_ERRNO != tc->memfd_create_exp_err)
diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create_common.c b/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
index 4cf3bd5..939b046 100644
--- a/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
+++ b/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
@@ -103,20 +103,56 @@ void check_ftruncate_fail(const char *filename, const int lineno,
"ftruncate(%d, %ld) failed as expected", fd, length);
}
-void assert_have_memfd_create(const char *filename, const int lineno)
+int get_mfd_all_available_flags(const char *filename, const int lineno)
{
- TEST(sys_memfd_create("dummy_call", 0));
+ unsigned int i;
+ int flag;
+ int flags2test[] = FLAGS_ALL_ARRAY_INITIALIZER;
+ int flags_available = 0;
+ int any_available = 0;
+
+ if (!IS_MFD_AVAILABLE_WITH_FLAGS(0)) {
+ tst_brk_(filename, lineno, TCONF,
+ "memfd_create(0) not implemented");
+ }
+
+ for (i = 0; i < ARRAY_SIZE(flags2test); i++) {
+ flag = flags2test[i];
+
+ TEST(IS_MFD_AVAILABLE_WITH_FLAGS(flag));
+ if (!TEST_RETURN) {
+ tst_res_(filename, lineno, TINFO | TTERRNO,
+ "memfd_create(%u) not implemented", flag);
+ } else {
+ flags_available |= flag;
+ any_available = 1;
+ }
+ }
+
+ if (!any_available) {
+ tst_brk_(filename, lineno, TCONF,
+ "memfd_create() not implemented@all");
+ }
+
+ return flags_available;
+}
+
+int is_mfd_available_with_flags(const char *filename, const int lineno,
+ unsigned int flags)
+{
+ TEST(sys_memfd_create("dummy_call", flags));
if (TEST_RETURN < 0) {
- if (TEST_ERRNO == EINVAL) {
- tst_brk_(filename, lineno, TCONF | TTERRNO,
- "memfd_create() not implemented");
+ if (TEST_ERRNO != EINVAL) {
+ tst_brk_(filename, lineno, TBROK | TTERRNO,
+ "memfd_create() failed");
}
- tst_brk_(filename, lineno, TBROK | TTERRNO,
- "memfd_create() failed");
+ return 0;
}
SAFE_CLOSE(TEST_RETURN);
+
+ return 1;
}
int check_mfd_new(const char *filename, const int lineno,
diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create_common.h b/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
index 6329ac3..616c7ef 100644
--- a/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
+++ b/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
@@ -20,10 +20,19 @@
#include <lapi/fcntl.h>
#include <lapi/memfd.h>
+#define COMMA ,
+#define BITWISE_OR |
+#define OPERATE_ON_FLAGS(x) MFD_CLOEXEC x MFD_ALLOW_SEALING
+#define FLAGS_ALL_ARRAY_INITIALIZER {OPERATE_ON_FLAGS(COMMA)}
+#define FLAGS_ALL_MASK (OPERATE_ON_FLAGS(BITWISE_OR))
+
#define MFD_DEF_SIZE 8192
-#define ASSERT_HAVE_MEMFD_CREATE() \
- assert_have_memfd_create(__FILE__, __LINE__)
+#define GET_MFD_ALL_AVAILABLE_FLAGS() \
+ get_mfd_all_available_flags(__FILE__, __LINE__)
+
+#define IS_MFD_AVAILABLE_WITH_FLAGS(flags) \
+ is_mfd_available_with_flags(__FILE__, __LINE__, (flags))
#define CHECK_MFD_NEW(name, sz, flags) \
check_mfd_new(__FILE__, __LINE__, (name), (sz), (flags))
@@ -89,7 +98,11 @@
#define CHECK_MFD_NON_GROWABLE_BY_WRITE(fd) \
check_mfd_non_growable_by_write(__FILE__, __LINE__, (fd))
-void assert_have_memfd_create(const char *filename, const int lineno);
+int is_mfd_available_with_flags(const char *filename, const int lineno,
+ unsigned int flags);
+
+int get_mfd_all_available_flags(const char *filename, const int lineno);
+
int sys_memfd_create(const char *name, unsigned int flags);
int check_fallocate(const char *filename, const int lineno, int fd,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [LTP] [PATCH] syscalls/memfd_create: test memfd flags separately
2017-06-01 11:34 Jakub Racek
@ 2017-06-06 13:15 ` Cyril Hrubis
2017-06-07 12:13 ` Jakub =?unknown-8bit?q?Ra=C4=8Dek?=
0 siblings, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2017-06-06 13:15 UTC (permalink / raw)
To: ltp
Hi!
> This patch fixes the test on some kernels where memfd_create doesn't
> have have sealing support. Calls to memfd_create are done so that we
> know what flags are available. Depending on that, some tests are skipped
> instead of incorrectly reporting breakage.
>
> Signed-off-by: Jakub Racek <jracek@redhat.com>
> ---
> .../kernel/syscalls/memfd_create/memfd_create01.c | 16 ++++++-
> .../kernel/syscalls/memfd_create/memfd_create02.c | 13 +++++-
> .../syscalls/memfd_create/memfd_create_common.c | 50 +++++++++++++++++++---
> .../syscalls/memfd_create/memfd_create_common.h | 19 ++++++--
> 4 files changed, 86 insertions(+), 12 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create01.c b/testcases/kernel/syscalls/memfd_create/memfd_create01.c
> index 54c817b..c5e76f2 100644
> --- a/testcases/kernel/syscalls/memfd_create/memfd_create01.c
> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create01.c
> @@ -22,6 +22,7 @@
>
> #define _GNU_SOURCE
>
> +#include <errno.h>
> #include "tst_test.h"
> #include "memfd_create_common.h"
>
> @@ -259,7 +260,20 @@ static void verify_memfd_create(unsigned int n)
>
> static void setup(void)
> {
> - ASSERT_HAVE_MEMFD_CREATE();
> +
> + int available_flags;
> +
> + /*
> + * For now, all tests in this file require MFD_ALLOW_SEALING flag
> + * to be implemented, even though that flag isn't always set when
> + * memfd is created. So don't check anything else and TCONF right away
> + * is this flag is missing.
> + */
> + available_flags = GET_MFD_ALL_AVAILABLE_FLAGS();
> + if ((available_flags & MFD_ALLOW_SEALING) != MFD_ALLOW_SEALING) {
> + tst_brk(TCONF | TTERRNO,
> + "memfd_create(%u) not implemented", MFD_ALLOW_SEALING);
> + }
Can't we call the IS_MFD_AVAILABLE_WITH_FLAGS(flag) here directly
instead? That would be more straightforward way to figure out if we
support sealing.
Also better name would be MFD_FLAGS_AVAILABLE() or something and the
function name to get all available flags should be shorther and to the
point as well, maybe MFD_ALL_AVAILABLE_FLAGS().
> }
>
> static struct tst_test test = {
> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create02.c b/testcases/kernel/syscalls/memfd_create/memfd_create02.c
> index d65e496..bc01e44 100644
> --- a/testcases/kernel/syscalls/memfd_create/memfd_create02.c
> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create02.c
> @@ -31,6 +31,8 @@
> static char buf[2048];
> static char term_buf[2048];
>
> +static int available_flags;
> +
> static const struct tcase {
> char *descr;
> char *memfd_name;
> @@ -62,7 +64,8 @@ static const struct tcase {
>
> static void setup(void)
> {
> - ASSERT_HAVE_MEMFD_CREATE();
> +
> + available_flags = GET_MFD_ALL_AVAILABLE_FLAGS();
>
> memset(buf, 0xff, sizeof(buf));
>
> @@ -73,8 +76,16 @@ static void setup(void)
> static void verify_memfd_create_errno(unsigned int n)
> {
> const struct tcase *tc;
> + int needed_flags;
>
> tc = &tcases[n];
> + needed_flags = tc->flags & FLAGS_ALL_MASK;
> +
> + if ((available_flags & needed_flags) != needed_flags) {
> + tst_res(TCONF, "test '%s' skipped, flag not implemented",
> + tc->descr);
> + return;
> + }
>
> TEST(sys_memfd_create(tc->memfd_name, tc->flags));
> if (TEST_ERRNO != tc->memfd_create_exp_err)
> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create_common.c b/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
> index 4cf3bd5..939b046 100644
> --- a/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
> @@ -103,20 +103,56 @@ void check_ftruncate_fail(const char *filename, const int lineno,
> "ftruncate(%d, %ld) failed as expected", fd, length);
> }
>
> -void assert_have_memfd_create(const char *filename, const int lineno)
> +int get_mfd_all_available_flags(const char *filename, const int lineno)
> {
> - TEST(sys_memfd_create("dummy_call", 0));
> + unsigned int i;
> + int flag;
> + int flags2test[] = FLAGS_ALL_ARRAY_INITIALIZER;
> + int flags_available = 0;
> + int any_available = 0;
> +
> + if (!IS_MFD_AVAILABLE_WITH_FLAGS(0)) {
> + tst_brk_(filename, lineno, TCONF,
> + "memfd_create(0) not implemented");
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(flags2test); i++) {
> + flag = flags2test[i];
> +
> + TEST(IS_MFD_AVAILABLE_WITH_FLAGS(flag));
Since we check errno in the is_mfd_...() function and call tst_brk_() on
failure there is no point in doing anything else here than:
if (is_mfd_...(flag))
flags_available |= flag;
> + if (!TEST_RETURN) {
> + tst_res_(filename, lineno, TINFO | TTERRNO,
> + "memfd_create(%u) not implemented", flag);
> + } else {
> + flags_available |= flag;
> + any_available = 1;
> + }
> + }
> +
> + if (!any_available) {
> + tst_brk_(filename, lineno, TCONF,
> + "memfd_create() not implemented at all");
> + }
Isn't this check useless now? Since we assert that at least 0 passed as
flags is supported at the start of this function we should just return
flags_available here which would be 0 in this case.
> + return flags_available;
> +}
> +
> +int is_mfd_available_with_flags(const char *filename, const int lineno,
> + unsigned int flags)
> +{
> + TEST(sys_memfd_create("dummy_call", flags));
> if (TEST_RETURN < 0) {
> - if (TEST_ERRNO == EINVAL) {
> - tst_brk_(filename, lineno, TCONF | TTERRNO,
> - "memfd_create() not implemented");
> + if (TEST_ERRNO != EINVAL) {
> + tst_brk_(filename, lineno, TBROK | TTERRNO,
> + "memfd_create() failed");
> }
>
> - tst_brk_(filename, lineno, TBROK | TTERRNO,
> - "memfd_create() failed");
> + return 0;
> }
>
> SAFE_CLOSE(TEST_RETURN);
> +
> + return 1;
> }
>
> int check_mfd_new(const char *filename, const int lineno,
> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create_common.h b/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
> index 6329ac3..616c7ef 100644
> --- a/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
> @@ -20,10 +20,19 @@
> #include <lapi/fcntl.h>
> #include <lapi/memfd.h>
>
> +#define COMMA ,
> +#define BITWISE_OR |
> +#define OPERATE_ON_FLAGS(x) MFD_CLOEXEC x MFD_ALLOW_SEALING
> +#define FLAGS_ALL_ARRAY_INITIALIZER {OPERATE_ON_FLAGS(COMMA)}
> +#define FLAGS_ALL_MASK (OPERATE_ON_FLAGS(BITWISE_OR))
Really?
Do you realize that just defining the FLAGS_ALL_MASK and array
initializer directly as MFD_CLOEXEC | MFD_ALLOW_SEALING and
{MFD_CLOEXEC, MFD_ALLOW_SEALING} is not only less complex but shorther
as well?
What do we need this macro obscurity for?
> #define MFD_DEF_SIZE 8192
>
> -#define ASSERT_HAVE_MEMFD_CREATE() \
> - assert_have_memfd_create(__FILE__, __LINE__)
> +#define GET_MFD_ALL_AVAILABLE_FLAGS() \
> + get_mfd_all_available_flags(__FILE__, __LINE__)
> +
> +#define IS_MFD_AVAILABLE_WITH_FLAGS(flags) \
> + is_mfd_available_with_flags(__FILE__, __LINE__, (flags))
>
> #define CHECK_MFD_NEW(name, sz, flags) \
> check_mfd_new(__FILE__, __LINE__, (name), (sz), (flags))
> @@ -89,7 +98,11 @@
> #define CHECK_MFD_NON_GROWABLE_BY_WRITE(fd) \
> check_mfd_non_growable_by_write(__FILE__, __LINE__, (fd))
>
> -void assert_have_memfd_create(const char *filename, const int lineno);
> +int is_mfd_available_with_flags(const char *filename, const int lineno,
> + unsigned int flags);
> +
> +int get_mfd_all_available_flags(const char *filename, const int lineno);
> +
> int sys_memfd_create(const char *name, unsigned int flags);
>
> int check_fallocate(const char *filename, const int lineno, int fd,
> --
> 1.8.3.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH] syscalls/memfd_create: test memfd flags separately
2017-06-06 13:15 ` Cyril Hrubis
@ 2017-06-07 12:13 ` Jakub =?unknown-8bit?q?Ra=C4=8Dek?=
0 siblings, 0 replies; 6+ messages in thread
From: Jakub =?unknown-8bit?q?Ra=C4=8Dek?= @ 2017-06-07 12:13 UTC (permalink / raw)
To: ltp
Hi,
On 06/06/2017 03:15 PM, Cyril Hrubis wrote:
> Hi!
>> This patch fixes the test on some kernels where memfd_create doesn't
>> have have sealing support. Calls to memfd_create are done so that we
>> know what flags are available. Depending on that, some tests are skipped
>> instead of incorrectly reporting breakage.
>>
>> Signed-off-by: Jakub Racek <jracek@redhat.com>
>> ---
>> .../kernel/syscalls/memfd_create/memfd_create01.c | 16 ++++++-
>> .../kernel/syscalls/memfd_create/memfd_create02.c | 13 +++++-
>> .../syscalls/memfd_create/memfd_create_common.c | 50 +++++++++++++++++++---
>> .../syscalls/memfd_create/memfd_create_common.h | 19 ++++++--
>> 4 files changed, 86 insertions(+), 12 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create01.c b/testcases/kernel/syscalls/memfd_create/memfd_create01.c
>> index 54c817b..c5e76f2 100644
>> --- a/testcases/kernel/syscalls/memfd_create/memfd_create01.c
>> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create01.c
>> @@ -22,6 +22,7 @@
>>
>> #define _GNU_SOURCE
>>
>> +#include <errno.h>
>> #include "tst_test.h"
>> #include "memfd_create_common.h"
>>
>> @@ -259,7 +260,20 @@ static void verify_memfd_create(unsigned int n)
>>
>> static void setup(void)
>> {
>> - ASSERT_HAVE_MEMFD_CREATE();
>> +
>> + int available_flags;
>> +
>> + /*
>> + * For now, all tests in this file require MFD_ALLOW_SEALING flag
>> + * to be implemented, even though that flag isn't always set when
>> + * memfd is created. So don't check anything else and TCONF right away
>> + * is this flag is missing.
>> + */
>> + available_flags = GET_MFD_ALL_AVAILABLE_FLAGS();
>> + if ((available_flags & MFD_ALLOW_SEALING) != MFD_ALLOW_SEALING) {
>> + tst_brk(TCONF | TTERRNO,
>> + "memfd_create(%u) not implemented", MFD_ALLOW_SEALING);
>> + }
>
> Can't we call the IS_MFD_AVAILABLE_WITH_FLAGS(flag) here directly
> instead? That would be more straightforward way to figure out if we
> support sealing.
>
> Also better name would be MFD_FLAGS_AVAILABLE() or something and the
> function name to get all available flags should be shorther and to the
> point as well, maybe MFD_ALL_AVAILABLE_FLAGS().
>
>> }
>>
>> static struct tst_test test = {
>> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create02.c b/testcases/kernel/syscalls/memfd_create/memfd_create02.c
>> index d65e496..bc01e44 100644
>> --- a/testcases/kernel/syscalls/memfd_create/memfd_create02.c
>> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create02.c
>> @@ -31,6 +31,8 @@
>> static char buf[2048];
>> static char term_buf[2048];
>>
>> +static int available_flags;
>> +
>> static const struct tcase {
>> char *descr;
>> char *memfd_name;
>> @@ -62,7 +64,8 @@ static const struct tcase {
>>
>> static void setup(void)
>> {
>> - ASSERT_HAVE_MEMFD_CREATE();
>> +
>> + available_flags = GET_MFD_ALL_AVAILABLE_FLAGS();
>>
>> memset(buf, 0xff, sizeof(buf));
>>
>> @@ -73,8 +76,16 @@ static void setup(void)
>> static void verify_memfd_create_errno(unsigned int n)
>> {
>> const struct tcase *tc;
>> + int needed_flags;
>>
>> tc = &tcases[n];
>> + needed_flags = tc->flags & FLAGS_ALL_MASK;
>> +
>> + if ((available_flags & needed_flags) != needed_flags) {
>> + tst_res(TCONF, "test '%s' skipped, flag not implemented",
>> + tc->descr);
>> + return;
>> + }
>>
>> TEST(sys_memfd_create(tc->memfd_name, tc->flags));
>> if (TEST_ERRNO != tc->memfd_create_exp_err)
>> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create_common.c b/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
>> index 4cf3bd5..939b046 100644
>> --- a/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
>> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create_common.c
>> @@ -103,20 +103,56 @@ void check_ftruncate_fail(const char *filename, const int lineno,
>> "ftruncate(%d, %ld) failed as expected", fd, length);
>> }
>>
>> -void assert_have_memfd_create(const char *filename, const int lineno)
>> +int get_mfd_all_available_flags(const char *filename, const int lineno)
>> {
>> - TEST(sys_memfd_create("dummy_call", 0));
>> + unsigned int i;
>> + int flag;
>> + int flags2test[] = FLAGS_ALL_ARRAY_INITIALIZER;
>> + int flags_available = 0;
>> + int any_available = 0;
>> +
>> + if (!IS_MFD_AVAILABLE_WITH_FLAGS(0)) {
>> + tst_brk_(filename, lineno, TCONF,
>> + "memfd_create(0) not implemented");
>> + }
>> +
>> + for (i = 0; i < ARRAY_SIZE(flags2test); i++) {
>> + flag = flags2test[i];
>> +
>> + TEST(IS_MFD_AVAILABLE_WITH_FLAGS(flag));
>
> Since we check errno in the is_mfd_...() function and call tst_brk_() on
> failure there is no point in doing anything else here than:
>
> if (is_mfd_...(flag))
> flags_available |= flag;
>
>> + if (!TEST_RETURN) {
>> + tst_res_(filename, lineno, TINFO | TTERRNO,
>> + "memfd_create(%u) not implemented", flag);
>> + } else {
>> + flags_available |= flag;
>> + any_available = 1;
>> + }
>> + }
>> +
>> + if (!any_available) {
>> + tst_brk_(filename, lineno, TCONF,
>> + "memfd_create() not implemented at all");
>> + }
>
> Isn't this check useless now? Since we assert that at least 0 passed as
> flags is supported at the start of this function we should just return
> flags_available here which would be 0 in this case.
>
>> + return flags_available;
>> +}
>> +
>> +int is_mfd_available_with_flags(const char *filename, const int lineno,
>> + unsigned int flags)
>> +{
>> + TEST(sys_memfd_create("dummy_call", flags));
>> if (TEST_RETURN < 0) {
>> - if (TEST_ERRNO == EINVAL) {
>> - tst_brk_(filename, lineno, TCONF | TTERRNO,
>> - "memfd_create() not implemented");
>> + if (TEST_ERRNO != EINVAL) {
>> + tst_brk_(filename, lineno, TBROK | TTERRNO,
>> + "memfd_create() failed");
>> }
>>
>> - tst_brk_(filename, lineno, TBROK | TTERRNO,
>> - "memfd_create() failed");
>> + return 0;
>> }
>>
>> SAFE_CLOSE(TEST_RETURN);
>> +
>> + return 1;
>> }
>>
>> int check_mfd_new(const char *filename, const int lineno,
>> diff --git a/testcases/kernel/syscalls/memfd_create/memfd_create_common.h b/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
>> index 6329ac3..616c7ef 100644
>> --- a/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
>> +++ b/testcases/kernel/syscalls/memfd_create/memfd_create_common.h
>> @@ -20,10 +20,19 @@
>> #include <lapi/fcntl.h>
>> #include <lapi/memfd.h>
>>
>> +#define COMMA ,
>> +#define BITWISE_OR |
>> +#define OPERATE_ON_FLAGS(x) MFD_CLOEXEC x MFD_ALLOW_SEALING
>> +#define FLAGS_ALL_ARRAY_INITIALIZER {OPERATE_ON_FLAGS(COMMA)}
>> +#define FLAGS_ALL_MASK (OPERATE_ON_FLAGS(BITWISE_OR))
>
> Really?
>
> Do you realize that just defining the FLAGS_ALL_MASK and array
> initializer directly as MFD_CLOEXEC | MFD_ALLOW_SEALING and
> {MFD_CLOEXEC, MFD_ALLOW_SEALING} is not only less complex but shorther
> as well?
>
It depends on what you see as simpler. I tried to avoid defining the
same flags on two lines (Single Point of Definition). If API ever
changes (e.g. more flags get added), then someone may easily change the
first line, but forget the second. But I get your point and I've added a
comment instead.
Thank you for your comments. Sending V3 now.
> What do we need this macro obscurity for?
>
>> #define MFD_DEF_SIZE 8192
>>
>> -#define ASSERT_HAVE_MEMFD_CREATE() \
>> - assert_have_memfd_create(__FILE__, __LINE__)
>> +#define GET_MFD_ALL_AVAILABLE_FLAGS() \
>> + get_mfd_all_available_flags(__FILE__, __LINE__)
>> +
>> +#define IS_MFD_AVAILABLE_WITH_FLAGS(flags) \
>> + is_mfd_available_with_flags(__FILE__, __LINE__, (flags))
>>
>> #define CHECK_MFD_NEW(name, sz, flags) \
>> check_mfd_new(__FILE__, __LINE__, (name), (sz), (flags))
>> @@ -89,7 +98,11 @@
>> #define CHECK_MFD_NON_GROWABLE_BY_WRITE(fd) \
>> check_mfd_non_growable_by_write(__FILE__, __LINE__, (fd))
>>
>> -void assert_have_memfd_create(const char *filename, const int lineno);
>> +int is_mfd_available_with_flags(const char *filename, const int lineno,
>> + unsigned int flags);
>> +
>> +int get_mfd_all_available_flags(const char *filename, const int lineno);
>> +
>> int sys_memfd_create(const char *name, unsigned int flags);
>>
>> int check_fallocate(const char *filename, const int lineno, int fd,
>> --
>> 1.8.3.1
>>
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp
>
--
Regards,
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-06-07 12:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-30 7:59 [LTP] [PATCH] syscalls/memfd_create: test memfd flags separately Jakub Racek
2017-05-31 10:56 ` Cyril Hrubis
2017-06-01 11:33 ` Jakub =?unknown-8bit?q?Ra=C4=8Dek?=
-- strict thread matches above, loose matches on Subject: below --
2017-06-01 11:34 Jakub Racek
2017-06-06 13:15 ` Cyril Hrubis
2017-06-07 12:13 ` Jakub =?unknown-8bit?q?Ra=C4=8Dek?=
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox