Skip to content

Commit

Permalink
m_option: initialize m_option_value union properly
Browse files Browse the repository at this point in the history
C standard says that `= {0}` activates and initializes first member of
union. We expect whole union to be zeroed, it is used as default value.

Initialize union with one zeroed default instance to ensure proper init.

Fixes: mpv-player#12711
  • Loading branch information
kasper93 authored and sfan5 committed Oct 23, 2023
1 parent 9074436 commit 1805681
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 16 deletions.
4 changes: 1 addition & 3 deletions options/m_config_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,6 @@ struct m_group_data {
uint64_t ts; // timestamp of the data copy
};

static const union m_option_value default_value = {0};

static void add_sub_group(struct m_config_shadow *shadow, const char *name_prefix,
int parent_group_index, int parent_ptr,
const struct m_sub_options *subopts);
Expand Down Expand Up @@ -241,7 +239,7 @@ const void *m_config_shadow_get_opt_default(struct m_config_shadow *shadow,
if (subopt->defaults)
return (char *)subopt->defaults + opt->offset;

return &default_value;
return &m_option_value_default;
}

void *m_config_cache_get_opt_data(struct m_config_cache *cache, int32_t id)
Expand Down
7 changes: 3 additions & 4 deletions options/m_config_frontend.c
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@ int m_config_set_option_cli(struct m_config *config, struct bstr name,
BSTR_P(name), BSTR_P(param), flags);
}

union m_option_value val = {0};
union m_option_value val = m_option_value_default;

// Some option types are "impure" and work on the existing data.
// (Prime examples: --vf-add, --sub-file)
Expand Down Expand Up @@ -783,7 +783,7 @@ int m_config_set_option_node(struct m_config *config, bstr name,

// Do this on an "empty" type to make setting the option strictly overwrite
// the old value, as opposed to e.g. appending to lists.
union m_option_value val = {0};
union m_option_value val = m_option_value_default;

if (data->format == MPV_FORMAT_STRING) {
bstr param = bstr0(data->u.string);
Expand Down Expand Up @@ -868,9 +868,8 @@ void m_config_print_option_list(const struct m_config *config, const char *name)
}
char *def = NULL;
const void *defptr = m_config_get_co_default(config, co);
const union m_option_value default_value = {0};
if (!defptr)
defptr = &default_value;
defptr = &m_option_value_default;
if (defptr)
def = m_option_pretty_print(opt, defptr);
if (def) {
Expand Down
5 changes: 5 additions & 0 deletions options/m_option.h
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,11 @@ union m_option_value {
struct m_channels channels;
};

// Keep fully zeroed instance of m_option_value to use as a default value, before
// any specific union member is used. C standard says that `= {0}` activates and
// initializes only the first member of the union, leaving padding bits undefined.
static const union m_option_value m_option_value_default;

////////////////////////////////////////////////////////////////////////////

struct m_option_action {
Expand Down
6 changes: 3 additions & 3 deletions options/m_property.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ static int m_property_multiply(struct mp_log *log,
const struct m_property *prop_list,
const char *property, double f, void *ctx)
{
union m_option_value val = {0};
union m_option_value val = m_option_value_default;
struct m_option opt = {0};
int r;

Expand Down Expand Up @@ -98,7 +98,7 @@ static int do_action(const struct m_property *prop_list, const char *name,
int m_property_do(struct mp_log *log, const struct m_property *prop_list,
const char *name, int action, void *arg, void *ctx)
{
union m_option_value val = {0};
union m_option_value val = m_option_value_default;
int r;

struct m_option opt = {0};
Expand Down Expand Up @@ -562,7 +562,7 @@ int m_property_read_list(int action, void *arg, int count,
r = get_item(n, M_PROPERTY_GET_TYPE, &opt, ctx);
if (r != M_PROPERTY_OK)
goto err;
union m_option_value val = {0};
union m_option_value val = m_option_value_default;
r = get_item(n, M_PROPERTY_GET, &val, ctx);
if (r != M_PROPERTY_OK)
goto err;
Expand Down
6 changes: 4 additions & 2 deletions player/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -1402,7 +1402,7 @@ static void getproperty_fn(void *arg)
struct getproperty_request *req = arg;
const struct m_option *type = get_mp_type_get(req->format);

union m_option_value xdata = {0};
union m_option_value xdata = m_option_value_default;
void *data = req->data ? req->data : &xdata;

int err = -1;
Expand Down Expand Up @@ -1560,6 +1560,8 @@ int mpv_observe_property(mpv_handle *ctx, uint64_t userdata,
.type = type,
.change_ts = 1, // force initial event
.refcount = 1,
.value = m_option_value_default,
.value_ret = m_option_value_default,
};
ctx->properties_change_ts += 1;
MP_TARRAY_APPEND(ctx, ctx->properties, ctx->num_properties, prop);
Expand Down Expand Up @@ -1682,7 +1684,7 @@ static void send_client_property_changes(struct mpv_handle *ctx)
bool changed = false;
if (prop->format) {
const struct m_option *type = prop->type;
union m_option_value val = {0};
union m_option_value val = m_option_value_default;
struct getproperty_request req = {
.mpctx = ctx->mpctx,
.name = prop->name,
Expand Down
8 changes: 4 additions & 4 deletions player/command.c
Original file line number Diff line number Diff line change
Expand Up @@ -3490,7 +3490,7 @@ static int mp_property_option_info(void *ctx, struct m_property *prop,
return M_PROPERTY_UNKNOWN;
const struct m_option *opt = co->opt;

union m_option_value def = {0};
union m_option_value def = m_option_value_default;
const void *def_ptr = m_config_get_co_default(mpctx->mconfig, co);
if (def_ptr && opt->type->size > 0)
memcpy(&def, def_ptr, opt->type->size);
Expand Down Expand Up @@ -4801,7 +4801,7 @@ static void cmd_cycle_values(void *p)
return;
}

union m_option_value curval = {0};
union m_option_value curval = m_option_value_default;
r = mp_property_do(name, M_PROPERTY_GET, &curval, mpctx);
if (r <= 0) {
show_property_status(cmd, name, r);
Expand All @@ -4810,7 +4810,7 @@ static void cmd_cycle_values(void *p)

int current = -1;
for (int n = first; n < cmd->num_args; n++) {
union m_option_value val = {0};
union m_option_value val = m_option_value_default;
if (m_option_parse(mpctx->log, &prop, bstr0(name),
bstr0(cmd->args[n].v.s), &val) < 0)
continue;
Expand Down Expand Up @@ -5214,7 +5214,7 @@ static void cmd_change_list(void *p)
return;
}

union m_option_value val = {0};
union m_option_value val = m_option_value_default;
if (mp_property_do(name, M_PROPERTY_GET, &val, mpctx) <= 0) {
set_osd_msg(mpctx, osdl, osd_duration, "Could not read: '%s'", name);
cmd->success = false;
Expand Down

0 comments on commit 1805681

Please sign in to comment.