Accessing elements in an empty va_list is undefined behaviour. Therefore, remove the variadicness from safe_* wrappers that always call the functions with the optional argument included.
Adapt the SAFE_OPEN, SAFE_OPENAT, SAFE_SEMCTL macros to handle the change by passing a default arguments if they're omitted.
v3: - Combined the open/openat patch with the semctl one into a single series. - Split the open/openat patch into two patches. - Introduced a new PATCH (i.e. 1/4) that implements a common handler chooser for open/openat/semctl cases. - Removed an unused va_list variable.
v2: - Added parenthesis around macro arguments
Review branch: https://git.morello-project.org/tudcre01/morello-linux-ltp/-/commits/review/...
Tudor Cretu (4): safe_macros: Introduce macro to avoid undefined behaviour in variadic safe functions safe_open: Fix undefined behaviour in vararg handling safe_openat: Fix undefined behaviour in vararg handling safe_semctl: Fix undefined behaviour in vararg handling
include/old/safe_macros.h | 12 ++++++++++-- include/safe_macros_fn.h | 3 ++- include/safe_vararg_macros.h | 37 ++++++++++++++++++++++++++++++++++++ include/tst_safe_file_at.h | 19 ++++++++++++++---- include/tst_safe_macros.h | 11 +++++++++-- include/tst_safe_sysv_ipc.h | 14 +++++++++++--- lib/safe_macros.c | 13 +------------ lib/tst_safe_file_at.c | 11 +++-------- lib/tst_safe_sysv_ipc.c | 10 +--------- 9 files changed, 89 insertions(+), 41 deletions(-) create mode 100644 include/safe_vararg_macros.h
Some safe functions are wrappers around functions with an optional parameter (e.g. open, openat, semctl). To access an element from an empty va_list is undefined behaviour. Introduce the macro SAFE_WRAPPER_COND_HANDLER that handles the variadicness of such wrappers by selecting the right handler for each case.
The intended usage is:
#define SAFE_FN(args, ...) \ SAFE_WRAPPER_COND_HANDLER(, ##__VA_ARGS__, \ SAFE_FN_HANDLER_1, \ SAFE_FN_HANLDER_0) \ ((args), ##__VA_ARGS__)
Subsequent patches would adapt the SAFE_* macros to avoid the undefined behaviour in the safe_* functions.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com --- include/safe_vararg_macros.h | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 include/safe_vararg_macros.h
diff --git a/include/safe_vararg_macros.h b/include/safe_vararg_macros.h new file mode 100644 index 000000000..cc95d2dd0 --- /dev/null +++ b/include/safe_vararg_macros.h @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2022 Arm Ltd. + * + * Internal helper functions for macros that handle safe wrappers for variadic + * functions. + * Do not use directly in test programs. + */ + +/* + * Some safe functions are wrappers around functions with an optional parameter + * (e.g. open, openat, semctl). To access an element from an empty va_list is + * undefined behaviour. This macro enables handling the variadicness of such + * wrappers by selecting the right handler for each case. The intended usage is: + * + * #define SAFE_FN(args, ...) \ + * SAFE_WRAPPER_COND_HANDLER(, ##__VA_ARGS__, \ + * SAFE_FN_HANDLER_1, \ + * SAFE_FN_HANLDER_0) \ + * ((args), ##__VA_ARGS__) + * + * Where SAFE_FN_HANDLER_0 is the handler used when the optional parameter is + * not specified and SAFE_FN_HANDLER_1 is the handler used when the optional + * parameter is specified. + * + * The way it works is that SAFE_WRAPPER_COND_HANDLER returns the third + * parameter passed. If SAFE_FN is called with an element in __VA_ARGS__, + * then SAFE_WRAPPER_COND_HANDLER just returns SAFE_FN_HANDLER_1. If SAFE_FN + * is called with an empty __VA_ARGS__, then the comma before the ‘##’ will be + * deleted and SAFE_FN_HANLDER_0 becomes the third argument. Finally, the chosen + * handler is called with the original arguments. + * + * Note: this macro only handles len(__VA_ARGS__) <= 1. + */ +#define SAFE_WRAPPER_COND_HANDLER(cond_empty_drop, cond_non_empty, SAFE_HELPER, ...) \ + SAFE_HELPER +
Hi Tudor,
On Tue, Nov 08, 2022 at 04:14:10PM +0000, Tudor Cretu wrote:
Some safe functions are wrappers around functions with an optional parameter (e.g. open, openat, semctl). To access an element from an
I think those are mostly (if not all) wrappers for syscalls so I guess we could re-phrase this as: Some of the safe functions providing wrappers for commonly used syscalls allow specifying an optional parameter ......
empty va_list is undefined behaviour. Introduce the macro SAFE_WRAPPER_COND_HANDLER that handles the variadicness of such
/that handles/that takes over handling the .......
wrappers by selecting the right handler for each case.
/for each case/for whether the optional argument has been provided or not ?
The intended usage is:
#define SAFE_FN(args, ...) \ SAFE_WRAPPER_COND_HANDLER(, ##__VA_ARGS__, \ SAFE_FN_HANDLER_1, \ SAFE_FN_HANLDER_0) \ ((args), ##__VA_ARGS__)
Subsequent patches would adapt the SAFE_* macros to avoid the undefined
relevant SAFE_* ....
behaviour in the safe_* functions.
the corresponding safe_* ....
Signed-off-by: Tudor Cretu tudor.cretu@arm.com
include/safe_vararg_macros.h | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 include/safe_vararg_macros.h
diff --git a/include/safe_vararg_macros.h b/include/safe_vararg_macros.h
Just wondering if it would not be better to have the file name prefixed with 'tst_', to align with ones like tst_safe_macros.h (some of the 'old' headers are already including the 'tst_*' ones so it should be good on that front)
new file mode 100644 index 000000000..cc95d2dd0 --- /dev/null +++ b/include/safe_vararg_macros.h @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: GPL-2.0 -or-later +/*
- Copyright (c) 2022 Arm Ltd.
- Internal helper functions for macros that handle safe wrappers for variadic
/helper functions/helpers ?
- functions.
We are actually ending up (with this series) with variadic macros, not the functions themselves, so maybe simply 'safe variadic wrappers for corresponding safe functions' ? (same would apply to comments below)
- Do not use directly in test programs.
- */
+/*
- Some safe functions are wrappers around functions with an optional parameter
/wrappers around functions/wrappers for syscalls ?
- (e.g. open, openat, semctl). To access an element from an empty va_list is
- undefined behaviour. This macro enables handling the variadicness of such
- wrappers by selecting the right handler for each case. The intended usage is:
/for each case/depending on whether the variadic argument has been supplied or not ?
- #define SAFE_FN(args, ...) \
SAFE_WRAPPER_COND_HANDLER(, ##__VA_ARGS__, \
SAFE_FN_HANDLER_1, \
SAFE_FN_HANLDER_0) \
((args), ##__VA_ARGS__)
- Where SAFE_FN_HANDLER_0 is the handler used when the optional parameter is
- not specified and SAFE_FN_HANDLER_1 is the handler used when the optional
- parameter is specified.
SAFE_FN_HANDLER_1 is the handler used otherwise. (to reduce repeating of parameter is specified ....)
- The way it works is that SAFE_WRAPPER_COND_HANDLER returns the third
- parameter passed. If SAFE_FN is called with an element in __VA_ARGS__,
/with an element/with a variadic argument
- then SAFE_WRAPPER_COND_HANDLER just returns SAFE_FN_HANDLER_1. If SAFE_FN
- is called with an empty __VA_ARGS__, then the comma before the ‘##’ will be
- deleted and SAFE_FN_HANLDER_0 becomes the third argument. Finally, the chosen
as a consequence the macro arguments will get shifted, with SAFE_FN_HANLDER_0 becoming the actual third argument
- handler is called with the original arguments.
So the last statement refers to safe macros using the SAFE_WRAPPER_COND_HANDLER. SAFE_WRAPPER_COND_HANDLER per se is not actually dealing with the arguments, it's there only to pick the right handler.
- Note: this macro only handles len(__VA_ARGS__) <= 1.
- */
+#define SAFE_WRAPPER_COND_HANDLER(cond_empty_drop, cond_non_empty, SAFE_HELPER, ...) \
- SAFE_HELPER
The naming here suggested in the previous thread was more of a illustrative purposes really. SAFE_WRAPPER_COND_HANDLER proves to be somewhat tediously too verbose so maybe we could use SAFE_COND_HANDLER, which would help getting the formatting more ...readable (?). Furthermore, as the first argument is really only a syntax guard (to get the preceding comma for ##__VA_ARGS__ ) we could give it more meaningful name like empty or void with the second one becoming cond_arg ? How does that sound ?
--- BR B.
-- 2.25.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
Hi Beata,
On 11-11-2022 14:45, Beata Michalska wrote:
Hi Tudor,
On Tue, Nov 08, 2022 at 04:14:10PM +0000, Tudor Cretu wrote:
Some safe functions are wrappers around functions with an optional parameter (e.g. open, openat, semctl). To access an element from an
I think those are mostly (if not all) wrappers for syscalls so I guess we could re-phrase this as: Some of the safe functions providing wrappers for commonly used syscalls allow specifying an optional parameter ......
empty va_list is undefined behaviour. Introduce the macro SAFE_WRAPPER_COND_HANDLER that handles the variadicness of such
/that handles/that takes over handling the .......
wrappers by selecting the right handler for each case.
/for each case/for whether the optional argument has been provided or not ?
The intended usage is:
#define SAFE_FN(args, ...) \ SAFE_WRAPPER_COND_HANDLER(, ##__VA_ARGS__, \ SAFE_FN_HANDLER_1, \ SAFE_FN_HANLDER_0) \ ((args), ##__VA_ARGS__)
Subsequent patches would adapt the SAFE_* macros to avoid the undefined
relevant SAFE_* ....behaviour in the safe_* functions.
the corresponding safe_* ....Signed-off-by: Tudor Cretu tudor.cretu@arm.com
include/safe_vararg_macros.h | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 include/safe_vararg_macros.h
diff --git a/include/safe_vararg_macros.h b/include/safe_vararg_macros.h
Just wondering if it would not be better to have the file name prefixed with 'tst_', to align with ones like tst_safe_macros.h (some of the 'old' headers are already including the 'tst_*' ones so it should be good on that front)
new file mode 100644 index 000000000..cc95d2dd0 --- /dev/null +++ b/include/safe_vararg_macros.h @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: GPL-2.0 -or-later +/*
- Copyright (c) 2022 Arm Ltd.
- Internal helper functions for macros that handle safe wrappers for variadic
/helper functions/helpers ?
- functions.
We are actually ending up (with this series) with variadic macros, not the functions themselves, so maybe simply 'safe variadic wrappers for corresponding safe functions' ? (same would apply to comments below)
- Do not use directly in test programs.
- */
+/*
- Some safe functions are wrappers around functions with an optional parameter
/wrappers around functions/wrappers for syscalls ?
- (e.g. open, openat, semctl). To access an element from an empty va_list is
- undefined behaviour. This macro enables handling the variadicness of such
- wrappers by selecting the right handler for each case. The intended usage is:
/for each case/depending on whether the variadic argument has been supplied or not ?
- #define SAFE_FN(args, ...) \
SAFE_WRAPPER_COND_HANDLER(, ##__VA_ARGS__, \
SAFE_FN_HANDLER_1, \
SAFE_FN_HANLDER_0) \
((args), ##__VA_ARGS__)
- Where SAFE_FN_HANDLER_0 is the handler used when the optional parameter is
- not specified and SAFE_FN_HANDLER_1 is the handler used when the optional
- parameter is specified.
SAFE_FN_HANDLER_1 is the handler used otherwise. (to reduce repeating of parameter is specified ....)
- The way it works is that SAFE_WRAPPER_COND_HANDLER returns the third
- parameter passed. If SAFE_FN is called with an element in __VA_ARGS__,
/with an element/with a variadic argument
- then SAFE_WRAPPER_COND_HANDLER just returns SAFE_FN_HANDLER_1. If SAFE_FN
- is called with an empty __VA_ARGS__, then the comma before the ‘##’ will be
- deleted and SAFE_FN_HANLDER_0 becomes the third argument. Finally, the chosen
as a consequence the macro arguments will get shifted, with SAFE_FN_HANLDER_0 becoming the actual third argument
- handler is called with the original arguments.
So the last statement refers to safe macros using the SAFE_WRAPPER_COND_HANDLER. SAFE_WRAPPER_COND_HANDLER per se is not actually dealing with the arguments, it's there only to pick the right handler.
- Note: this macro only handles len(__VA_ARGS__) <= 1.
- */
+#define SAFE_WRAPPER_COND_HANDLER(cond_empty_drop, cond_non_empty, SAFE_HELPER, ...) \
- SAFE_HELPER
The naming here suggested in the previous thread was more of a illustrative purposes really. SAFE_WRAPPER_COND_HANDLER proves to be somewhat tediously too verbose so maybe we could use SAFE_COND_HANDLER, which would help getting the formatting more ...readable (?). Furthermore, as the first argument is really only a syntax guard (to get the preceding comma for ##__VA_ARGS__ ) we could give it more meaningful name like empty or void with the second one becoming cond_arg ? How does that sound ?
Many thanks for the detailed review! The suggestions sound good! I've updated the series, feel free to further suggest improvement.
Thanks, Tudor
BR B.
-- 2.25.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
Accessing elements in an empty va_list is undefined behaviour. Therefore, remove the variadicness from safe_open as it always calls open with the mode argument included.
Adapt the SAFE_OPEN macro to handle the change by passing a default argument of 0 to mode if it's omitted.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com --- include/old/safe_macros.h | 12 ++++++++++-- include/safe_macros_fn.h | 3 ++- include/tst_safe_macros.h | 11 +++++++++-- lib/safe_macros.c | 13 +------------ 4 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/include/old/safe_macros.h b/include/old/safe_macros.h index fb1d7a110..83ca78b54 100644 --- a/include/old/safe_macros.h +++ b/include/old/safe_macros.h @@ -17,6 +17,7 @@ #define __SAFE_MACROS_H__
#include "safe_macros_fn.h" +#include "safe_vararg_macros.h" #include "old_safe_stdio.h" #include "old_safe_net.h"
@@ -59,9 +60,16 @@ #define SAFE_MUNMAP(cleanup_fn, addr, length) \ safe_munmap(__FILE__, __LINE__, (cleanup_fn), (addr), (length))
-#define SAFE_OPEN(cleanup_fn, pathname, oflags, ...) \ +#define _SAFE_OPEN_3(cleanup_fn, pathname, oflags) \ + safe_open(__FILE__, __LINE__, (cleanup_fn), (pathname), (oflags), 0) + +#define _SAFE_OPEN_4(cleanup_fn, pathname, oflags, mode) \ safe_open(__FILE__, __LINE__, (cleanup_fn), (pathname), (oflags), \ - ##__VA_ARGS__) + (mode)) + +#define SAFE_OPEN(cleanup_fn, pathname, oflags, ...) \ + SAFE_WRAPPER_COND_HANDLER(, ##__VA_ARGS__, _SAFE_OPEN_4, _SAFE_OPEN_3) \ + ((cleanup_fn), (pathname), (oflags), ##__VA_ARGS__)
#define SAFE_PIPE(cleanup_fn, fildes) \ safe_pipe(__FILE__, __LINE__, cleanup_fn, (fildes)) diff --git a/include/safe_macros_fn.h b/include/safe_macros_fn.h index 3df952811..e84759839 100644 --- a/include/safe_macros_fn.h +++ b/include/safe_macros_fn.h @@ -62,7 +62,8 @@ int safe_munmap(const char *file, const int lineno, void (*cleanup_fn)(void), void *addr, size_t length);
int safe_open(const char *file, const int lineno, - void (*cleanup_fn)(void), const char *pathname, int oflags, ...); + void (*cleanup_fn)(void), const char *pathname, int oflags, + mode_t mode);
int safe_pipe(const char *file, const int lineno, void (*cleanup_fn)(void), int fildes[2]); diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h index 81c4b0844..11116a1e0 100644 --- a/include/tst_safe_macros.h +++ b/include/tst_safe_macros.h @@ -22,6 +22,7 @@ #include <grp.h>
#include "safe_macros_fn.h" +#include "safe_vararg_macros.h" #include "tst_cmd.h"
int safe_access(const char *filename, const int lineno, const char *pathname, @@ -86,9 +87,15 @@ void *safe_realloc(const char *file, const int lineno, void *ptr, size_t size); #define SAFE_MUNMAP(addr, length) \ safe_munmap(__FILE__, __LINE__, NULL, (addr), (length))
+#define _SAFE_OPEN_2(pathname, oflags) \ + safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), 0) + +#define _SAFE_OPEN_3(pathname, oflags, mode) \ + safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), (mode)) + #define SAFE_OPEN(pathname, oflags, ...) \ - safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), \ - ##__VA_ARGS__) + SAFE_WRAPPER_COND_HANDLER(, ##__VA_ARGS__, _SAFE_OPEN_3, _SAFE_OPEN_2) \ + ((pathname), (oflags), ##__VA_ARGS__)
#define SAFE_PIPE(fildes) \ safe_pipe(__FILE__, __LINE__, NULL, (fildes)) diff --git a/lib/safe_macros.c b/lib/safe_macros.c index a5b6bc504..40891e841 100644 --- a/lib/safe_macros.c +++ b/lib/safe_macros.c @@ -234,20 +234,9 @@ int safe_munmap(const char *file, const int lineno, void (*cleanup_fn) (void), }
int safe_open(const char *file, const int lineno, void (*cleanup_fn) (void), - const char *pathname, int oflags, ...) + const char *pathname, int oflags, mode_t mode) { - va_list ap; int rval; - mode_t mode; - - va_start(ap, oflags); - - /* Android's NDK's mode_t is smaller than an int, which results in - * SIGILL here when passing the mode_t type. - */ - mode = va_arg(ap, int); - - va_end(ap);
rval = open(pathname, oflags, mode);
On Tue, Nov 08, 2022 at 04:14:11PM +0000, Tudor Cretu wrote:
Accessing elements in an empty va_list is undefined behaviour. Therefore, remove the variadicness from safe_open as it always calls open with the mode argument included.
Adapt the SAFE_OPEN macro to handle the change by passing a default argument of 0 to mode if it's omitted.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com
include/old/safe_macros.h | 12 ++++++++++-- include/safe_macros_fn.h | 3 ++- include/tst_safe_macros.h | 11 +++++++++-- lib/safe_macros.c | 13 +------------ 4 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/include/old/safe_macros.h b/include/old/safe_macros.h index fb1d7a110..83ca78b54 100644 --- a/include/old/safe_macros.h +++ b/include/old/safe_macros.h @@ -17,6 +17,7 @@ #define __SAFE_MACROS_H__ #include "safe_macros_fn.h" +#include "safe_vararg_macros.h" #include "old_safe_stdio.h" #include "old_safe_net.h" @@ -59,9 +60,16 @@ #define SAFE_MUNMAP(cleanup_fn, addr, length) \ safe_munmap(__FILE__, __LINE__, (cleanup_fn), (addr), (length)) -#define SAFE_OPEN(cleanup_fn, pathname, oflags, ...) \ +#define _SAFE_OPEN_3(cleanup_fn, pathname, oflags) \
- safe_open(__FILE__, __LINE__, (cleanup_fn), (pathname), (oflags), 0)
+#define _SAFE_OPEN_4(cleanup_fn, pathname, oflags, mode) \ safe_open(__FILE__, __LINE__, (cleanup_fn), (pathname), (oflags), \
##__VA_ARGS__)
(mode))+#define SAFE_OPEN(cleanup_fn, pathname, oflags, ...) \
- SAFE_WRAPPER_COND_HANDLER(, ##__VA_ARGS__, _SAFE_OPEN_4, _SAFE_OPEN_3) \
((cleanup_fn), (pathname), (oflags), ##__VA_ARGS__)
Could we potentially align the arguments (to opening brackets), breaking the lines even further ? (same applies to other places where the macro is being used)
--- BR B.
#define SAFE_PIPE(cleanup_fn, fildes) \ safe_pipe(__FILE__, __LINE__, cleanup_fn, (fildes)) diff --git a/include/safe_macros_fn.h b/include/safe_macros_fn.h index 3df952811..e84759839 100644 --- a/include/safe_macros_fn.h +++ b/include/safe_macros_fn.h @@ -62,7 +62,8 @@ int safe_munmap(const char *file, const int lineno, void (*cleanup_fn)(void), void *addr, size_t length); int safe_open(const char *file, const int lineno,
void (*cleanup_fn)(void), const char *pathname, int oflags, ...);
void (*cleanup_fn)(void), const char *pathname, int oflags,mode_t mode);int safe_pipe(const char *file, const int lineno, void (*cleanup_fn)(void), int fildes[2]); diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h index 81c4b0844..11116a1e0 100644 --- a/include/tst_safe_macros.h +++ b/include/tst_safe_macros.h @@ -22,6 +22,7 @@ #include <grp.h> #include "safe_macros_fn.h" +#include "safe_vararg_macros.h" #include "tst_cmd.h" int safe_access(const char *filename, const int lineno, const char *pathname, @@ -86,9 +87,15 @@ void *safe_realloc(const char *file, const int lineno, void *ptr, size_t size); #define SAFE_MUNMAP(addr, length) \ safe_munmap(__FILE__, __LINE__, NULL, (addr), (length)) +#define _SAFE_OPEN_2(pathname, oflags) \
- safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), 0)
+#define _SAFE_OPEN_3(pathname, oflags, mode) \
- safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), (mode))
#define SAFE_OPEN(pathname, oflags, ...) \
- safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), \
##__VA_ARGS__)
- SAFE_WRAPPER_COND_HANDLER(, ##__VA_ARGS__, _SAFE_OPEN_3, _SAFE_OPEN_2) \
- ((pathname), (oflags), ##__VA_ARGS__)
#define SAFE_PIPE(fildes) \ safe_pipe(__FILE__, __LINE__, NULL, (fildes)) diff --git a/lib/safe_macros.c b/lib/safe_macros.c index a5b6bc504..40891e841 100644 --- a/lib/safe_macros.c +++ b/lib/safe_macros.c @@ -234,20 +234,9 @@ int safe_munmap(const char *file, const int lineno, void (*cleanup_fn) (void), } int safe_open(const char *file, const int lineno, void (*cleanup_fn) (void),
const char *pathname, int oflags, ...)
const char *pathname, int oflags, mode_t mode){
- va_list ap; int rval;
- mode_t mode;
- va_start(ap, oflags);
- /* Android's NDK's mode_t is smaller than an int, which results in
* SIGILL here when passing the mode_t type.*/- mode = va_arg(ap, int);
- va_end(ap);
rval = open(pathname, oflags, mode); -- 2.25.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
Accessing elements in an empty va_list is undefined behaviour. Therefore, remove the variadicness from safe_openat as it always calls openat with the mode argument included.
Adapt the SAFE_OPENAT macro to handle the change by passing a default argument of 0 to mode if it's omitted.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com --- include/tst_safe_file_at.h | 19 +++++++++++++++---- lib/tst_safe_file_at.c | 11 +++-------- 2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/include/tst_safe_file_at.h b/include/tst_safe_file_at.h index 8df34227f..dabed370d 100644 --- a/include/tst_safe_file_at.h +++ b/include/tst_safe_file_at.h @@ -8,10 +8,21 @@
#include <sys/types.h> #include <stdarg.h> +#include "safe_vararg_macros.h"
-#define SAFE_OPENAT(dirfd, path, oflags, ...) \ - safe_openat(__FILE__, __LINE__, \ - (dirfd), (path), (oflags), ## __VA_ARGS__) +#define _SAFE_OPENAT_3(dirfd, path, oflags) \ + safe_openat(__FILE__, __LINE__, \ + (dirfd), (path), (oflags), 0) + +#define _SAFE_OPENAT_4(dirfd, path, oflags, mode) \ + safe_openat(__FILE__, __LINE__, \ + (dirfd), (path), (oflags), (mode)) + +#define SAFE_OPENAT(dirfd, path, oflags, ...) \ + SAFE_WRAPPER_COND_HANDLER(, ##__VA_ARGS__, \ + _SAFE_OPENAT_4, \ + _SAFE_OPENAT_3) \ + ((dirfd), (path), (oflags), ##__VA_ARGS__)
#define SAFE_FILE_READAT(dirfd, path, buf, nbyte) \ safe_file_readat(__FILE__, __LINE__, \ @@ -29,7 +40,7 @@ const char *tst_decode_fd(const int fd) __attribute__((warn_unused_result));
int safe_openat(const char *const file, const int lineno, const int dirfd, - const char *const path, const int oflags, ...) + const char *const path, const int oflags, const mode_t mode) __attribute__((nonnull, warn_unused_result));
ssize_t safe_file_readat(const char *const file, const int lineno, diff --git a/lib/tst_safe_file_at.c b/lib/tst_safe_file_at.c index ca8ef2f68..9582f17d8 100644 --- a/lib/tst_safe_file_at.c +++ b/lib/tst_safe_file_at.c @@ -33,15 +33,10 @@ const char *tst_decode_fd(const int fd) }
int safe_openat(const char *const file, const int lineno, - const int dirfd, const char *const path, const int oflags, ...) + const int dirfd, const char *const path, const int oflags, + const mode_t mode) { - va_list ap; int fd; - mode_t mode; - - va_start(ap, oflags); - mode = va_arg(ap, int); - va_end(ap);
fd = openat(dirfd, path, oflags, mode); if (fd > -1) @@ -58,7 +53,7 @@ ssize_t safe_file_readat(const char *const file, const int lineno, const int dirfd, const char *const path, char *const buf, const size_t nbyte) { - int fd = safe_openat(file, lineno, dirfd, path, O_RDONLY); + int fd = safe_openat(file, lineno, dirfd, path, O_RDONLY, 0); ssize_t rval;
if (fd < 0)
On Tue, Nov 08, 2022 at 04:14:12PM +0000, Tudor Cretu wrote:
Accessing elements in an empty va_list is undefined behaviour. Therefore, remove the variadicness from safe_openat as it always calls openat with the mode argument included.
Adapt the SAFE_OPENAT macro to handle the change by passing a default argument of 0 to mode if it's omitted.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com
include/tst_safe_file_at.h | 19 +++++++++++++++---- lib/tst_safe_file_at.c | 11 +++-------- 2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/include/tst_safe_file_at.h b/include/tst_safe_file_at.h index 8df34227f..dabed370d 100644 --- a/include/tst_safe_file_at.h +++ b/include/tst_safe_file_at.h @@ -8,10 +8,21 @@ #include <sys/types.h> #include <stdarg.h> +#include "safe_vararg_macros.h" -#define SAFE_OPENAT(dirfd, path, oflags, ...) \
- safe_openat(__FILE__, __LINE__, \
(dirfd), (path), (oflags), ## __VA_ARGS__)+#define _SAFE_OPENAT_3(dirfd, path, oflags) \
- safe_openat(__FILE__, __LINE__, \
(dirfd), (path), (oflags), 0)
Neat: This one and the one above should now fit into single line
--- BR B.
+#define _SAFE_OPENAT_4(dirfd, path, oflags, mode) \
- safe_openat(__FILE__, __LINE__, \
(dirfd), (path), (oflags), (mode))+#define SAFE_OPENAT(dirfd, path, oflags, ...) \
- SAFE_WRAPPER_COND_HANDLER(, ##__VA_ARGS__, \
_SAFE_OPENAT_4, \_SAFE_OPENAT_3) \- ((dirfd), (path), (oflags), ##__VA_ARGS__)
#define SAFE_FILE_READAT(dirfd, path, buf, nbyte) \ safe_file_readat(__FILE__, __LINE__, \ @@ -29,7 +40,7 @@ const char *tst_decode_fd(const int fd) __attribute__((warn_unused_result)); int safe_openat(const char *const file, const int lineno, const int dirfd,
const char *const path, const int oflags, ...)
const char *const path, const int oflags, const mode_t mode)ssize_t safe_file_readat(const char *const file, const int lineno, diff --git a/lib/tst_safe_file_at.c b/lib/tst_safe_file_at.c index ca8ef2f68..9582f17d8 100644 --- a/lib/tst_safe_file_at.c +++ b/lib/tst_safe_file_at.c @@ -33,15 +33,10 @@ const char *tst_decode_fd(const int fd) } int safe_openat(const char *const file, const int lineno,
const int dirfd, const char *const path, const int oflags, ...)
const int dirfd, const char *const path, const int oflags,const mode_t mode){
- va_list ap; int fd;
- mode_t mode;
- va_start(ap, oflags);
- mode = va_arg(ap, int);
- va_end(ap);
fd = openat(dirfd, path, oflags, mode); if (fd > -1) @@ -58,7 +53,7 @@ ssize_t safe_file_readat(const char *const file, const int lineno, const int dirfd, const char *const path, char *const buf, const size_t nbyte) {
- int fd = safe_openat(file, lineno, dirfd, path, O_RDONLY);
- int fd = safe_openat(file, lineno, dirfd, path, O_RDONLY, 0); ssize_t rval;
if (fd < 0) -- 2.25.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
Accessing elements in an empty va_list is undefined behaviour. Therefore, remove the variadicness from safe_semctl as it always calls semctl with the union semun argument included.
Adapt the SAFE_SEMCTL macro to handle the change by passing a zero-initialised union semun if it's omitted.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com --- include/tst_safe_sysv_ipc.h | 14 +++++++++++--- lib/tst_safe_sysv_ipc.c | 10 +--------- 2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/tst_safe_sysv_ipc.h b/include/tst_safe_sysv_ipc.h index 7804ce192..41af7aeaa 100644 --- a/include/tst_safe_sysv_ipc.h +++ b/include/tst_safe_sysv_ipc.h @@ -10,6 +10,8 @@ #include <sys/msg.h> #include <sys/shm.h> #include <sys/sem.h> +#include "lapi/sem.h" +#include "safe_vararg_macros.h"
int safe_msgget(const char *file, const int lineno, key_t key, int msgflg); #define SAFE_MSGGET(key, msgflg) \ @@ -58,10 +60,16 @@ int safe_semget(const char *file, const int lineno, key_t key, int nsems, safe_semget(__FILE__, __LINE__, (key), (nsems), (semflg))
int safe_semctl(const char *file, const int lineno, int semid, int semnum, - int cmd, ...); + int cmd, union semun un); +#define _SAFE_SEMCTL_3(semid, semnum, cmd) \ + safe_semctl(__FILE__, __LINE__, (semid), (semnum), (cmd), (union semun){0}) + +#define _SAFE_SEMCTL_4(semid, semnum, cmd, un) \ + safe_semctl(__FILE__, __LINE__, (semid), (semnum), (cmd), (un)) + #define SAFE_SEMCTL(semid, semnum, cmd, ...) ({ \ - int tst_ret_ = safe_semctl(__FILE__, __LINE__, (semid), (semnum), \ - (cmd), ##__VA_ARGS__); \ + int tst_ret_ = SAFE_WRAPPER_COND_HANDLER(, ##__VA_ARGS__, _SAFE_SEMCTL_4, \ + _SAFE_SEMCTL_3) ((semid), (semnum), (cmd), ##__VA_ARGS__); \ (semid) = ((cmd) == IPC_RMID ? -1 : (semid)); \ tst_ret_; })
diff --git a/lib/tst_safe_sysv_ipc.c b/lib/tst_safe_sysv_ipc.c index 5eaa82539..f99f6db5e 100644 --- a/lib/tst_safe_sysv_ipc.c +++ b/lib/tst_safe_sysv_ipc.c @@ -228,17 +228,9 @@ int safe_semget(const char *file, const int lineno, key_t key, int nsems, }
int safe_semctl(const char *file, const int lineno, int semid, int semnum, - int cmd, ...) + int cmd, union semun un) { int rval; - va_list va; - union semun un; - - va_start(va, cmd); - - un = va_arg(va, union semun); - - va_end(va);
rval = semctl(semid, semnum, cmd, un);
On Tue, Nov 08, 2022 at 04:14:13PM +0000, Tudor Cretu wrote:
Accessing elements in an empty va_list is undefined behaviour. Therefore, remove the variadicness from safe_semctl as it always calls semctl with the union semun argument included.
Adapt the SAFE_SEMCTL macro to handle the change by passing a zero-initialised union semun if it's omitted.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com
include/tst_safe_sysv_ipc.h | 14 +++++++++++--- lib/tst_safe_sysv_ipc.c | 10 +--------- 2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/tst_safe_sysv_ipc.h b/include/tst_safe_sysv_ipc.h index 7804ce192..41af7aeaa 100644 --- a/include/tst_safe_sysv_ipc.h +++ b/include/tst_safe_sysv_ipc.h @@ -10,6 +10,8 @@ #include <sys/msg.h> #include <sys/shm.h> #include <sys/sem.h> +#include "lapi/sem.h" +#include "safe_vararg_macros.h" int safe_msgget(const char *file, const int lineno, key_t key, int msgflg); #define SAFE_MSGGET(key, msgflg) \ @@ -58,10 +60,16 @@ int safe_semget(const char *file, const int lineno, key_t key, int nsems, safe_semget(__FILE__, __LINE__, (key), (nsems), (semflg)) int safe_semctl(const char *file, const int lineno, int semid, int semnum,
int cmd, ...);
int cmd, union semun un);
New line ?
+#define _SAFE_SEMCTL_3(semid, semnum, cmd) \
- safe_semctl(__FILE__, __LINE__, (semid), (semnum), (cmd), (union semun){0})
+#define _SAFE_SEMCTL_4(semid, semnum, cmd, un) \
- safe_semctl(__FILE__, __LINE__, (semid), (semnum), (cmd), (un))
#define SAFE_SEMCTL(semid, semnum, cmd, ...) ({ \
- int tst_ret_ = safe_semctl(__FILE__, __LINE__, (semid), (semnum), \
(cmd), ##__VA_ARGS__); \
- int tst_ret_ = SAFE_WRAPPER_COND_HANDLER(, ##__VA_ARGS__, _SAFE_SEMCTL_4, \
_SAFE_SEMCTL_3) ((semid), (semnum), (cmd), ##__VA_ARGS__); \
Aligning arguments, similar to other patches in this series.
--- BR B.
(semid) = ((cmd) == IPC_RMID ? -1 : (semid)); \ tst_ret_; }) diff --git a/lib/tst_safe_sysv_ipc.c b/lib/tst_safe_sysv_ipc.c index 5eaa82539..f99f6db5e 100644 --- a/lib/tst_safe_sysv_ipc.c +++ b/lib/tst_safe_sysv_ipc.c @@ -228,17 +228,9 @@ int safe_semget(const char *file, const int lineno, key_t key, int nsems, } int safe_semctl(const char *file, const int lineno, int semid, int semnum,
int cmd, ...)
int cmd, union semun un){ int rval;
- va_list va;
- union semun un;
- va_start(va, cmd);
- un = va_arg(va, union semun);
- va_end(va);
rval = semctl(semid, semnum, cmd, un); -- 2.25.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
linux-morello-ltp@op-lists.linaro.org
-
Beata Michalska -
Tudor Cretu