shared/extract-word: replace enum with unsigned int to avoid undefined behaviour

keszybz · keszybz · commit e4d85dbb3d5a · 2017-04-25T21:12:52.000-04:00
../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.
diff --git a/src/basic/extract-word.c b/src/basic/extract-word.c
@@ -241,7 +241,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
@@ -32,4 +32,4 @@ typedef enum ExtractFlags {
 
 int extract_first_word(const char **p, char **ret, const char *separators, ExtractFlags flags);
 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_;
+int extract_many_words(const char **p, const char *separators, unsigned flags, ...) _sentinel_;
