diff options
| author | Tobias Brunner <tobias@strongswan.org> | 2017-09-14 15:31:00 +0200 |
|---|---|---|
| committer | Tobias Brunner <tobias@strongswan.org> | 2017-09-18 12:07:26 +0200 |
| commit | 7cecc98e8ad11702e14e649eb62efa6125b8c31e (patch) | |
| tree | 6806ae4c0e33423ff2eb5b92f9eb5d78fe4c0d7b | |
| parent | 46a62f01264add0518a6729ea0a30b1aa610491b (diff) | |
| download | strongswan-7cecc98e8ad11702e14e649eb62efa6125b8c31e.tar.bz2 strongswan-7cecc98e8ad11702e14e649eb62efa6125b8c31e.tar.xz | |
settings: Fix possible undefined behavior with va_start() and bool
This fixes compilation with -Werror when using Clang 4.0 (but not 3.9)
and possibly prevents undefined behavior.
According to the C standard the following applies to the second
parameter of the va_start() macro (subclause 7.16.1.4, paragraph 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.
Because bool is usually just 1 byte and therefore smaller than int (i.e.
the result of default argument promotion) its use as last argument before
... might result in undefined behavior. This theoretically can also
apply to enums as a compiler may use a smaller base type than int.
Since Clang 3.9 (currently in use on Travis by default) a warning is
issued about this, however, that version did not yet compare the actual
size of the argument's type, causing warnings where they are not
warranted (basically for all cases where enum types are used for the
last argument). This was apparently fixed with Clang 4.0, which only
warns about this use of bool with va_start(), which makes sense.
| -rw-r--r-- | src/libstrongswan/settings/settings.c | 6 | ||||
| -rw-r--r-- | src/libstrongswan/settings/settings.h | 4 |
2 files changed, 6 insertions, 4 deletions
diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c index 2a92d523b..cdf3e704b 100644 --- a/src/libstrongswan/settings/settings.c +++ b/src/libstrongswan/settings/settings.c @@ -494,11 +494,12 @@ inline bool settings_value_as_bool(char *value, bool def) } METHOD(settings_t, get_bool, bool, - private_settings_t *this, char *key, bool def, ...) + private_settings_t *this, char *key, int def, ...) { char *value; va_list args; + /* we can't use bool for def due to this call */ va_start(args, def); value = find_value(this, this->top, key, args); va_end(args); @@ -665,9 +666,10 @@ METHOD(settings_t, set_str, void, } METHOD(settings_t, set_bool, void, - private_settings_t *this, char *key, bool value, ...) + private_settings_t *this, char *key, int value, ...) { va_list args; + /* we can't use bool for value due to this call */ va_start(args, value); set_value(this, this->top, key, args, value ? "1" : "0"); va_end(args); diff --git a/src/libstrongswan/settings/settings.h b/src/libstrongswan/settings/settings.h index eec5ece6c..28cde4876 100644 --- a/src/libstrongswan/settings/settings.h +++ b/src/libstrongswan/settings/settings.h @@ -173,7 +173,7 @@ struct settings_t { * @param ... argument list for key * @return value of the key */ - bool (*get_bool)(settings_t *this, char *key, bool def, ...); + bool (*get_bool)(settings_t *this, char *key, int def, ...); /** * Get an integer value. @@ -221,7 +221,7 @@ struct settings_t { * @param value value to set * @param ... argument list for key */ - void (*set_bool)(settings_t *this, char *key, bool value, ...); + void (*set_bool)(settings_t *this, char *key, int value, ...); /** * Set an integer value. |