m_options: more typesafety

Change the option definition macros so that they cause compiler warnings
if the type of the referenced option struct member doesn't match the
type implied by the macro. The compiler warning printed isn't very
telling, but it's better than silently invoking undefined behavior by
violating the C strict aliasing rules.

Also fix some minor cases that violate the type rules. For the option
"no-aspect" we have to add a new option type to handle it properly.

Some option types are hard to check, so we don't in these cases.

4 files changed, 73 insertions, 30 deletions

diff --git a/core/cfg-mplayer.h b/core/cfg-mplayer.h
index 57dd5253c8..22017f930b 100644
--- a/core/cfg-mplayer.h
+++ b/core/cfg-mplayer.h
@@ -456,8 +456,7 @@ const m_option_t common_opts[] = {
     {"sws", &sws_flags, CONF_TYPE_INT, 0, 0, 2, NULL},
     {"ssf", (void *) scaler_filter_conf, CONF_TYPE_SUBCONFIG, 0, 0, 0, NULL},
     OPT_FLOATRANGE("aspect", movie_aspect, 0, 0.1, 10.0),
-    // xxx: this aliases int with float, which is very evil (but works in this case)
-    OPT_FLAG_STORE("no-aspect", movie_aspect, 0, 0),
+    OPT_FLOAT_STORE("no-aspect", movie_aspect, 0, 0),
 
     OPT_FLAG_CONSTANTS("flip", flip, 0, 0, 1),
 
diff --git a/core/m_option.c b/core/m_option.c
index 507d6cc2c7..6d011ee141 100644
--- a/core/m_option.c
+++ b/core/m_option.c
@@ -179,6 +179,34 @@ const m_option_type_t m_option_type_store = {
     .parse = parse_store,
 };
 
+// Same for float types
+
+#undef VAL
+#define VAL(x) (*(float *)(x))
+
+static int parse_store_float(const m_option_t *opt, struct bstr name,
+                             struct bstr param, void *dst)
+{
+    if (param.len == 0 || bstrcasecmp0(param, "yes") == 0) {
+        if (dst)
+            VAL(dst) = opt->max;
+        return 0;
+    } else {
+        mp_msg(MSGT_CFGPARSER, MSGL_ERR,
+               "Invalid parameter for %.*s flag: %.*s\n",
+               BSTR_P(name), BSTR_P(param));
+        return M_OPT_DISALLOW_PARAM;
+    }
+}
+
+const m_option_type_t m_option_type_float_store = {
+    // can only be activated
+    .name  = "Flag",
+    .size  = sizeof(float),
+    .flags = M_OPT_TYPE_OPTIONAL_PARAM,
+    .parse = parse_store_float,
+};
+
 // Integer
 
 #undef VAL
diff --git a/core/m_option.h b/core/m_option.h
index 90e41b1447..6cbe739b54 100644
--- a/core/m_option.h
+++ b/core/m_option.h
@@ -37,6 +37,7 @@ struct m_struct_st;
 // Simple types
 extern const m_option_type_t m_option_type_flag;
 extern const m_option_type_t m_option_type_store;
+extern const m_option_type_t m_option_type_float_store;
 extern const m_option_type_t m_option_type_int;
 extern const m_option_type_t m_option_type_int64;
 extern const m_option_type_t m_option_type_intpair;
@@ -186,8 +187,10 @@ struct m_sub_options {
 union m_option_value {
     int flag; // not the C type "bool"!
     int store;
+    float float_store;
     int int_;
     int64_t int64;
+    int intpair[2];
     float float_;
     double double_;
     char *string;
@@ -472,7 +475,17 @@ static inline void m_option_free(const m_option_t *opt, void *dst)
         opt->type->free(dst);
 }
 
-/*@}*/
+// Cause a compilation warning if typeof(expr) != type.
+// Should be used with pointer types only.
+#define MP_EXPECT_TYPE(type, expr) (0 ? (type)0 : (expr))
+
+// This behaves like offsetof(type, member), but will cause a compilation
+// warning if typeof(member) != expected_member_type.
+// It uses some trickery to make it compile as expression.
+#define MP_CHECKED_OFFSETOF(type, member, expected_member_type)             \
+    (offsetof(type, member) + (0 && MP_EXPECT_TYPE(expected_member_type*,   \
+                                                   &((type*)0)->member)))
+
 
 #define OPTION_LIST_SEPARATOR ','
 
@@ -485,7 +498,9 @@ static inline void m_option_free(const m_option_t *opt, void *dst)
 #define OPTDEF_STR(s) .defval = (void *)&(char * const){s}
 #define OPTDEF_INT(i) .defval = (void *)&(const int){i}
 
-#define OPT_GENERAL(optname, varname, flagv, ...) {.name = optname, .flags = flagv, .new = 1, .offset = offsetof(OPT_BASE_STRUCT, varname), __VA_ARGS__}
+#define OPT_GENERAL(ctype, optname, varname, flagv, ...) {.name = optname, .flags = flagv, .new = 1, .offset = MP_CHECKED_OFFSETOF(OPT_BASE_STRUCT, varname, ctype), __VA_ARGS__}
+
+#define OPT_GENERAL_NOTYPE(optname, varname, flagv, ...) {.name = optname, .flags = flagv, .new = 1, .offset = offsetof(OPT_BASE_STRUCT, varname), __VA_ARGS__}
 
 /* The OPT_FLAG_CONSTANTS->OPT_FLAG_CONSTANTS_ kind of redirection exists to
  * make the code fully standard-conforming: the C standard requires that
@@ -493,34 +508,35 @@ static inline void m_option_free(const m_option_t *opt, void *dst)
  * 0). Thus the first OPT_FLAG_CONSTANTS is a wrapper which just adds one
  * argument to ensure __VA_ARGS__ is not empty when calling the next macro.
  */
-#define OPT_FLAG(...) OPT_GENERAL(__VA_ARGS__, .type = &m_option_type_flag, .max = 1)
+#define OPT_FLAG(...) OPT_GENERAL(int, __VA_ARGS__, .type = &m_option_type_flag, .max = 1)
 #define OPT_FLAG_CONSTANTS(...) OPT_FLAG_CONSTANTS_(__VA_ARGS__, .type = &m_option_type_flag)
-#define OPT_FLAG_CONSTANTS_(optname, varname, flags, offvalue, value, ...) OPT_GENERAL(optname, varname, flags, .min = offvalue, .max = value, __VA_ARGS__)
-#define OPT_FLAG_STORE(optname, varname, flags, value) OPT_GENERAL(optname, varname, flags, .max = value, .type = &m_option_type_store)
-#define OPT_STRINGLIST(...) OPT_GENERAL(__VA_ARGS__, .type = &m_option_type_string_list)
-#define OPT_PATHLIST(...) OPT_GENERAL(__VA_ARGS__, .type = &m_option_type_string_list, .priv = (void *)&(const char){OPTION_PATH_SEPARATOR})
-#define OPT_INT(...) OPT_GENERAL(__VA_ARGS__, .type = &m_option_type_int)
-#define OPT_INTRANGE(...) OPT_RANGE_(__VA_ARGS__, .type = &m_option_type_int)
-#define OPT_RANGE_(optname, varname, flags, minval, maxval, ...) OPT_GENERAL(optname, varname, (flags) | CONF_RANGE, .min = minval, .max = maxval, __VA_ARGS__)
-#define OPT_INTPAIR(...) OPT_GENERAL(__VA_ARGS__, .type = &m_option_type_intpair)
-#define OPT_FLOAT(...) OPT_GENERAL(__VA_ARGS__, .type = &m_option_type_float)
-#define OPT_FLOATRANGE(...) OPT_RANGE_(__VA_ARGS__, .type = &m_option_type_float)
-#define OPT_STRING(...) OPT_GENERAL(__VA_ARGS__, .type = &m_option_type_string)
-#define OPT_SETTINGSLIST(optname, varname, flags, objlist) OPT_GENERAL(optname, varname, flags, .type = &m_option_type_obj_settings_list, .priv = objlist)
-#define OPT_AUDIOFORMAT(...) OPT_GENERAL(__VA_ARGS__, .type = &m_option_type_afmt)
+#define OPT_FLAG_CONSTANTS_(optname, varname, flags, offvalue, value, ...) OPT_GENERAL(int, optname, varname, flags, .min = offvalue, .max = value, __VA_ARGS__)
+#define OPT_FLAG_STORE(optname, varname, flags, value) OPT_GENERAL(int, optname, varname, flags, .max = value, .type = &m_option_type_store)
+#define OPT_FLOAT_STORE(optname, varname, flags, value) OPT_GENERAL(float, optname, varname, flags, .max = value, .type = &m_option_type_float_store)
+#define OPT_STRINGLIST(...) OPT_GENERAL(char**, __VA_ARGS__, .type = &m_option_type_string_list)
+#define OPT_PATHLIST(...) OPT_GENERAL(char**, __VA_ARGS__, .type = &m_option_type_string_list, .priv = (void *)&(const char){OPTION_PATH_SEPARATOR})
+#define OPT_INT(...) OPT_GENERAL(int, __VA_ARGS__, .type = &m_option_type_int)
+#define OPT_INTRANGE(...) OPT_RANGE_(int, __VA_ARGS__, .type = &m_option_type_int)
+#define OPT_RANGE_(ctype, optname, varname, flags, minval, maxval, ...) OPT_GENERAL(ctype, optname, varname, (flags) | CONF_RANGE, .min = minval, .max = maxval, __VA_ARGS__)
+#define OPT_INTPAIR(...) OPT_GENERAL_NOTYPE(__VA_ARGS__, .type = &m_option_type_intpair)
+#define OPT_FLOAT(...) OPT_GENERAL(float, __VA_ARGS__, .type = &m_option_type_float)
+#define OPT_FLOATRANGE(...) OPT_RANGE_(float, __VA_ARGS__, .type = &m_option_type_float)
+#define OPT_STRING(...) OPT_GENERAL(char*, __VA_ARGS__, .type = &m_option_type_string)
+#define OPT_SETTINGSLIST(optname, varname, flags, objlist) OPT_GENERAL(m_obj_settings_t*, optname, varname, flags, .type = &m_option_type_obj_settings_list, .priv = objlist)
+#define OPT_AUDIOFORMAT(...) OPT_GENERAL(int, __VA_ARGS__, .type = &m_option_type_afmt)
 #define OPT_HELPER_REMOVEPAREN(...) __VA_ARGS__
 #define M_CHOICES(choices) .priv = (void *)&(const struct m_opt_choice_alternatives[]){OPT_HELPER_REMOVEPAREN choices, {NULL}}
 #define OPT_CHOICE(...) OPT_CHOICE_(__VA_ARGS__, .type = &m_option_type_choice)
-#define OPT_CHOICE_(optname, varname, flags, choices, ...) OPT_GENERAL(optname, varname, flags, M_CHOICES(choices), __VA_ARGS__)
+#define OPT_CHOICE_(optname, varname, flags, choices, ...) OPT_GENERAL(int, optname, varname, flags, M_CHOICES(choices), __VA_ARGS__)
 // Union of choices and an int range. The choice values can be included in the
 // int range, or be completely separate - both works.
 #define OPT_CHOICE_OR_INT(...) OPT_CHOICE_OR_INT_(__VA_ARGS__, .type = &m_option_type_choice)
-#define OPT_CHOICE_OR_INT_(optname, varname, flags, minval, maxval, choices, ...) OPT_GENERAL(optname, varname, (flags) | CONF_RANGE, .min = minval, .max = maxval, M_CHOICES(choices), __VA_ARGS__)
-#define OPT_TIME(...) OPT_GENERAL(__VA_ARGS__, .type = &m_option_type_time)
-#define OPT_REL_TIME(...) OPT_GENERAL(__VA_ARGS__, .type = &m_option_type_rel_time)
-#define OPT_COLOR(...) OPT_GENERAL(__VA_ARGS__, .type = &m_option_type_color)
-#define OPT_GEOMETRY(...) OPT_GENERAL(__VA_ARGS__, .type = &m_option_type_geometry)
-#define OPT_SIZE_BOX(...) OPT_GENERAL(__VA_ARGS__, .type = &m_option_type_size_box)
+#define OPT_CHOICE_OR_INT_(optname, varname, flags, minval, maxval, choices, ...) OPT_GENERAL(int, optname, varname, (flags) | CONF_RANGE, .min = minval, .max = maxval, M_CHOICES(choices), __VA_ARGS__)
+#define OPT_TIME(...) OPT_GENERAL(double, __VA_ARGS__, .type = &m_option_type_time)
+#define OPT_REL_TIME(...) OPT_GENERAL(struct m_rel_time, __VA_ARGS__, .type = &m_option_type_rel_time)
+#define OPT_COLOR(...) OPT_GENERAL(struct m_color, __VA_ARGS__, .type = &m_option_type_color)
+#define OPT_GEOMETRY(...) OPT_GENERAL(struct m_geometry, __VA_ARGS__, .type = &m_option_type_geometry)
+#define OPT_SIZE_BOX(...) OPT_GENERAL(struct m_geometry, __VA_ARGS__, .type = &m_option_type_size_box)
 
 #define OPT_TRACKCHOICE(name, var) OPT_CHOICE_OR_INT(name, var, 0, 0, 8190, ({"no", -2}, {"auto", -1}))
 
@@ -530,7 +546,7 @@ static inline void m_option_free(const m_option_t *opt, void *dst)
 // If name is "", add the sub-options directly instead.
 // varname refers to the field, that must be a pointer to a field described by
 // the subconf struct.
-#define OPT_SUBSTRUCT(name, varname, subconf, flagv) OPT_GENERAL(name, varname, flagv, .type = &m_option_type_subconfig_struct, .priv = (void*)&subconf)
+#define OPT_SUBSTRUCT(name, varname, subconf, flagv) OPT_GENERAL_NOTYPE(name, varname, flagv, .type = &m_option_type_subconfig_struct, .priv = (void*)&subconf)
 
 #define OPT_BASE_STRUCT struct MPOpts
 
diff --git a/core/options.h b/core/options.h
index f23ff51e44..a5f4c4023b 100644
--- a/core/options.h
+++ b/core/options.h
@@ -158,9 +158,9 @@ typedef struct MPOpts {
     } lavc_param;
 
     struct lavfdopts {
-        unsigned int probesize;
+        int probesize;
         int probescore;
-        unsigned int analyzeduration;
+        int analyzeduration;
         char *format;
         char *cryptokey;
         char *avopt;
@@ -169,8 +169,8 @@ typedef struct MPOpts {
     struct input_conf {
         char *config_file;
         int key_fifo_size;
-        unsigned int ar_delay;
-        unsigned int ar_rate;
+        int ar_delay;
+        int ar_rate;
         char *js_dev;
         char *ar_dev;
         char *in_file;