openposix: mmap/21-1: adjust the test to work with MAP_SHARED_VALIDATE
PATCH 1/1 openposix: mmap/21-1: adjust the test to work with MAP_SHARED_VALIDATE
Linux commit 50a8e840d050 ("mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags") introduced the MAP_SHARED_VALIDATE flag, which is similar to MAP_SHARED but
provides additional semantics. So on we need to make sure that the invalid
value of 'flags' in the test is generated taking into account this new flag.
--- .../conformance/interfaces/mmap/21-1.c | 20 +++++++++++++++++++- 1 files changed, 19 insertions(+), 1 deletions(-)
testcases/open_posix_testsuite/conformance/interfaces/mmap/21-1.c +19 -1
@@ -27,6 +27,24 @@#include <errno.h> #include "posixtest.h" +int is_valid (int flag) +{ +if (flag ==MAP_SHARED || flag ==MAP_PRIVATE || flag ==MAP_FIXED ) +return 1 ; + +#ifdef __linux__ + +#ifndef MAP_SHARED_VALIDATE +#define MAP_SHARED_VALIDATE 0x03 +#endif + +if (flag ==MAP_SHARED_VALIDATE ) +return 1 ; +#endif + +return 0 ; +} +int main (void ) {char tmpfname[256 ];
@@ -51,7 +69,7 @@ int main(void) } flag =MAP_SHARED ; -while (flag ==MAP_SHARED || flag ==MAP_PRIVATE || flag ==MAP_FIXED ) +while (is_valid (flag)) flag++; pa =mmap (NULL , size,PROT_READ |PROT_WRITE , flag, fd,0 );
Signature · 1 line
1.7.1
PATCH 1/1 openposix: mmap/21-1: adjust the test to work with MAP_SHARED_VALIDATE
Hi!
Quoted 7 lines
testcases/open_posix_testsuite/conformance/interfaces/mmap/21-1.c +7
-0+int is_valid (int flag) +{ +if (flag ==MAP_SHARED || flag ==MAP_PRIVATE || flag ==MAP_FIXED ) +return 1 ; + +#ifdef __linux__ +
testcases/open_posix_testsuite/conformance/interfaces/mmap/21-1.c line 46
Quoted 10 lines
testcases/open_posix_testsuite/conformance/interfaces/mmap/21-1.c +10
-0+#ifndef MAP_SHARED_VALIDATE +#define MAP_SHARED_VALIDATE 0x03 +#endif + +if (flag ==MAP_SHARED_VALIDATE ) +return 1 ; +#endif + +return 0 ; +}
Hmm, this probably does not scale up, what about setting the flag to have all its bits set (i.e. passing ~0 to the mmap()) from the start instead?
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH 1/1 openposix: mmap/21-1: adjust the test to work with MAP_SHARED_VALIDATE
Hi.
Quoted 14 lines · Cyril Hrubis
Hi!
Quoted 13 lines
testcases/open_posix_testsuite/conformance/interfaces/mmap/21-1.c +13
-0+int is_valid (int flag) +{ +if (flag ==MAP_SHARED || flag ==MAP_PRIVATE || flag ==MAP_FIXED ) +return 1 ; + +#ifdef __linux__ + +#ifndef MAP_SHARED_VALIDATE +#define MAP_SHARED_VALIDATE 0x03 +#endif + +if (flag ==MAP_SHARED_VALIDATE ) +return 1 ;
Quoted 9 lines
testcases/open_posix_testsuite/conformance/interfaces/mmap/21-1.c line 46
Quoted 4 lines
testcases/open_posix_testsuite/conformance/interfaces/mmap/21-1.c +4
-0+#endif + +return 0 ; +}Hmm, this probably does not scale up, what about setting the flag to have all its bits set (i.e. passing ~0 to the
mmap()) from the start instead?
In main() we search for the first value of 'flag' for which the following statement is not true:
flag == MAP_SHARED || flag == MAP_PRIVATE || flag == MAP_FIXED || flag == MAP_SHARED_VALIDATE
where the last part is Linux-specific and likely needs an #ifdef.
We are not interested if flag has MAP_SHARED or other bits set, but we are interested when flag _equals_ MAP_SHARED or other values.
In order to make the code more readable (IMHO) I decided to move this check into a separate function.
I don't fully understand your proposal. Could you, please, elaborate a bit more?
Thanks.
PATCH 1/1 openposix: mmap/21-1: adjust the test to work with MAP_SHARED_VALIDATE
Hi!
Quoted 31 lines
Quoted 13 lines
Quoted 13 lines
testcases/open_posix_testsuite/conformance/interfaces/mmap/21-1.c +13
-0+int is_valid (int flag) +{ +if (flag ==MAP_SHARED || flag ==MAP_PRIVATE || flag ==MAP_FIXED ) +return 1 ; + +#ifdef __linux__ + +#ifndef MAP_SHARED_VALIDATE +#define MAP_SHARED_VALIDATE 0x03 +#endif + +if (flag ==MAP_SHARED_VALIDATE ) +return 1 ;Quoted 9 lines
testcases/open_posix_testsuite/conformance/interfaces/mmap/21-1.c line 46
Quoted 4 lines
testcases/open_posix_testsuite/conformance/interfaces/mmap/21-1.c +4
-0+#endif + +return 0 ; +}Hmm, this probably does not scale up, what about setting the flag to have all its bits set (i.e. passing ~0 to the
mmap()) from the start instead?In
main()we search for the first value of 'flag' for which the following statement is not true:flag ==
MAP_SHARED|| flag ==MAP_PRIVATE|| flag ==MAP_FIXED|| flag ==MAP_SHARED_VALIDATEwhere the last part is Linux-specific and likely needs an #ifdef.
Quoted 8 lines
We are not interested if flag has
MAP_SHAREDor other bits set, but we are interested when flag _equals_MAP_SHAREDor other values.In order to make the code more readable (IMHO) I decided to move this check into a separate function.
I don't fully understand your proposal. Could you, please, elaborate a bit more?
We are trying to trigger EINVAL by passing an invalid flags value to mmap. And given that the flags are likely to be allocated in increasing manner starting with known flag and increasing it while it's equal to other known flags will break again sooner or later and we will end up adding more and more special cases for different systems.
Hence I suggest to use a value that is unlikely to be valid flag (all bits set) and use that for the testing.
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH 1/1 openposix: mmap/21-1: use all bits set as the invalid flag
The test is trying to trigger EINVAL by passing an invalid flags
value to mmap(). Linux commit 50a8e840d050 ("mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags") introduced a new flag (MAP_SHARED_VALIDATE) and that flag may not be the last one.
So instead of adding an 'if' for each new flag we use a value with all bits set (which is unlikely to be a valid flag).
--- .../conformance/interfaces/mmap/21-1.c | 6 +----- 1 files changed, 1 insertions(+), 5 deletions(-)
testcases/open_posix_testsuite/conformance/interfaces/mmap/21-1.c +1 -5
@@ -33,7 +33,7 @@ int main(void)void *pa;size_t size =1024 ; -int flag; +int flag = ~0 ;int fd;snprintf (tmpfname,sizeof (tmpfname),"/tmp/pts_mmap_21_1_%d" ,getpid ());
@@ -50,10 +50,6 @@ int main(void)return PTS_UNRESOLVED ; } - flag =MAP_SHARED ; -while (flag ==MAP_SHARED || flag ==MAP_PRIVATE || flag ==MAP_FIXED ) - flag++; - pa =mmap (NULL , size,PROT_READ |PROT_WRITE , flag, fd,0 );if (pa ==MAP_FAILED && errno ==EINVAL ) {printf ("Test PASSED\n" );
Signature · 1 line
1.7.1
PATCH 1/1 openposix: mmap/21-1: use all bits set as the invalid flag
Hi!
Quoted 6 lines
The test is trying to trigger EINVAL by passing an invalid flags
value tommap(). Linux commit50a8e840d050("mm: introduceMAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags") introduced a new flag (MAP_SHARED_VALIDATE) and that flag may not be the last one.So instead of adding an 'if' for each new flag we use a value with all bits set (which is unlikely to be a valid flag).
Acked, thanks.
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH 1/1 openposix: mmap/21-1: use all bits set as the invalid flag
Quoted 10 lines · Cyril Hrubis
Hi!
Quoted 6 lines
The test is trying to trigger EINVAL by passing an invalid flags
value tommap(). Linux commit50a8e840d050("mm: introduceMAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags") introduced a new flag (MAP_SHARED_VALIDATE) and that flag may not be the last one.So instead of adding an 'if' for each new flag we use a value with all bits set (which is unlikely to be a valid flag).
Acked, thanks.
Thank you. Committed.