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.

diff --git a/src/libstrongswan/settings/settings.c b/src/libstrongswan/settings/settings.c
index 2a92d523ba80ef4f84afaff06a6256a33771f463..cdf3e704bfc06c4d9a66a70a8d5de37dc52930a6 100644 (file)
--- 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 eec5ece6c84bc4be323b00112692409e605b60a3..28cde48769081f6b3b90dbfe53778b32e1b8c4de 100644 (file)
--- 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.