From 21d58b174835912a6c578d4ea389f49223c2d804 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 22 Apr 2017 19:04:02 -0400 Subject: [PATCH] shared/extract-word: replace enum with unsigned int to avoid undefined behaviour ../src/basic/extract-word.c:255:22: warning: passing an object that undergoes default argument promotion to 'va_start' has undefined behavior [-Wvarargs] va_start(ap, flags); ^ ../src/basic/extract-word.c:244:77: note: parameter of type 'ExtractFlags' (aka 'enum ExtractFlags') is declared here int extract_many_words(const char **p, const char *separators, ExtractFlags flags, ...) { ^ ../src/basic/extract-word.c:286:22: warning: passing an object that undergoes default argument promotion to 'va_start' has undefined behavior [-Wvarargs] va_start(ap, flags); ^ ../src/basic/extract-word.c:244:77: note: parameter of type 'ExtractFlags' (aka 'enum ExtractFlags') is declared here int extract_many_words(const char **p, const char *separators, ExtractFlags flags, ...) { ^ 2 warnings generated. I think the relevant part of C99 is 6.7.2.2 Enumeration specifiers: Each enumerated type shall be compatible with char, a signed integer type, or an unsigned integer type. The choice of type is implementation-defined, but shall be capable of representing the values of all the members of the enumeration. and 7.16.1.4: The parameter parmN is the identifier of the rightmost parameter in the variable parameter list in the function definition (the one just before the ...). If the parameter parmN is declared with the register storage class, with a function or array type, or with a type that is not compatible with the type that results after application of the default argument promotions, the behavior is undefined. This might cause a real issue if the compiler chooses something that is not an integer for ExtractFlags. Rework the code to avoid the warning, but add an assert_cc in a large-valued ExtractFlags element is ever defined and the type is bumped to something wider than an int. --- src/basic/extract-word.c | 7 ++++++- src/basic/extract-word.h | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/basic/extract-word.c b/src/basic/extract-word.c index 9d33c7f63..6f2959efd 100644 --- a/src/basic/extract-word.c +++ b/src/basic/extract-word.c @@ -242,7 +242,12 @@ int extract_first_word_and_warn( return log_syntax(unit, LOG_ERR, filename, line, r, "Unable to decode word \"%s\", ignoring: %m", rvalue); } -int extract_many_words(const char **p, const char *separators, ExtractFlags flags, ...) { +/* We pass ExtractFlags as unsigned int (to avoid undefined behaviour when passing + * an object that undergoes default argument promotion as an argument to va_start). + * Let's make sure that ExtractFlags fits into an unsigned int. */ +assert_cc(sizeof(enum ExtractFlags) <= sizeof(unsigned)); + +int extract_many_words(const char **p, const char *separators, unsigned flags, ...) { va_list ap; char **l; int n = 0, i, c, r; diff --git a/src/basic/extract-word.h b/src/basic/extract-word.h index aecbb101e..36006a5b6 100644 --- a/src/basic/extract-word.h +++ b/src/basic/extract-word.h @@ -33,5 +33,5 @@ typedef enum ExtractFlags { int extract_first_word(const char **p, char **ret, const char *separators, ExtractFlags flags); #if 0 /// UNNEEDED by elogind int extract_first_word_and_warn(const char **p, char **ret, const char *separators, ExtractFlags flags, const char *unit, const char *filename, unsigned line, const char *rvalue); -int extract_many_words(const char **p, const char *separators, ExtractFlags flags, ...) _sentinel_; #endif // 0 +int extract_many_words(const char **p, const char *separators, unsigned flags, ...) _sentinel_; -- 2.30.2