fix(options): use a union for def_val (#27169)

Problem:
APIs get wrong boolean option default values on big-endian platforms.

Solution:
Use a union for def_val.
Cannot use OptVal or OptValData yet as it needs to have the same types
as option variables.
This commit is contained in:
zeertzjq 2024-01-24 12:27:38 +08:00 committed by GitHub
parent 65bfa86efe
commit c8a27bae3f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 55 additions and 59 deletions

View File

@ -125,31 +125,27 @@ local function get_cond(c, base_string)
return cond_string return cond_string
end end
local value_dumpers = {
['function'] = function(v)
return v()
end,
string = cstr,
boolean = function(v)
return v and 'true' or 'false'
end,
number = function(v)
return ('%iL'):format(v)
end,
['nil'] = function(_)
return '0'
end,
}
local get_value = function(v)
return '(void *) ' .. value_dumpers[type(v)](v)
end
local get_defaults = function(d, n) local get_defaults = function(d, n)
if d == nil then if d == nil then
error("option '" .. n .. "' should have a default value") error("option '" .. n .. "' should have a default value")
end end
return get_value(d)
local value_dumpers = {
['function'] = function(v)
return v()
end,
string = function(v)
return '.string=' .. cstr(v)
end,
boolean = function(v)
return '.boolean=' .. (v and 'true' or 'false')
end,
number = function(v)
return ('.number=%iL'):format(v)
end,
}
return value_dumpers[type(d)](d)
end end
--- @type {[1]:string,[2]:string}[] --- @type {[1]:string,[2]:string}[]
@ -213,11 +209,11 @@ local function dump_option(i, o)
if o.defaults.condition then if o.defaults.condition then
w(get_cond(o.defaults.condition)) w(get_cond(o.defaults.condition))
end end
w(' .def_val=' .. get_defaults(o.defaults.if_true, o.full_name)) w(' .def_val' .. get_defaults(o.defaults.if_true, o.full_name))
if o.defaults.condition then if o.defaults.condition then
if o.defaults.if_false then if o.defaults.if_false then
w('#else') w('#else')
w(' .def_val=' .. get_defaults(o.defaults.if_false, o.full_name)) w(' .def_val' .. get_defaults(o.defaults.if_false, o.full_name))
end end
w('#endif') w('#endif')
end end

View File

@ -174,7 +174,7 @@ static int p_paste_dep_opts[] = {
void set_init_tablocal(void) void set_init_tablocal(void)
{ {
// susy baka: cmdheight calls itself OPT_GLOBAL but is really tablocal! // susy baka: cmdheight calls itself OPT_GLOBAL but is really tablocal!
p_ch = (OptInt)(intptr_t)options[kOptCmdheight].def_val; p_ch = options[kOptCmdheight].def_val.number;
} }
/// Initialize the 'shell' option to a default value. /// Initialize the 'shell' option to a default value.
@ -275,13 +275,8 @@ static void set_init_default_cdpath(void)
} }
} }
buf[j] = NUL; buf[j] = NUL;
OptIndex opt_idx = kOptCdpath; options[kOptCdpath].def_val.string = buf;
if (opt_idx != kOptInvalid) { options[kOptCdpath].flags |= P_DEF_ALLOCED;
options[opt_idx].def_val = buf;
options[opt_idx].flags |= P_DEF_ALLOCED;
} else {
xfree(buf); // cannot happen
}
xfree(cdpath); xfree(cdpath);
} }
@ -309,9 +304,9 @@ static void set_init_expand_env(void)
p = xstrdup(p); p = xstrdup(p);
*(char **)opt->var = p; *(char **)opt->var = p;
if (opt->flags & P_DEF_ALLOCED) { if (opt->flags & P_DEF_ALLOCED) {
xfree(opt->def_val); xfree(opt->def_val.string);
} }
opt->def_val = p; opt->def_val.string = p;
opt->flags |= P_DEF_ALLOCED; opt->flags |= P_DEF_ALLOCED;
} }
} }
@ -438,19 +433,19 @@ static void set_option_default(const OptIndex opt_idx, int opt_flags)
// Use set_string_option_direct() for local options to handle freeing and allocating the // Use set_string_option_direct() for local options to handle freeing and allocating the
// value. // value.
if (opt->indir != PV_NONE) { if (opt->indir != PV_NONE) {
set_string_option_direct(opt_idx, opt->def_val, opt_flags, 0); set_string_option_direct(opt_idx, opt->def_val.string, opt_flags, 0);
} else { } else {
if (flags & P_ALLOCED) { if (flags & P_ALLOCED) {
free_string_option(*(char **)(varp)); free_string_option(*(char **)(varp));
} }
*(char **)varp = opt->def_val; *(char **)varp = opt->def_val.string;
opt->flags &= ~P_ALLOCED; opt->flags &= ~P_ALLOCED;
} }
} else if (option_has_type(opt_idx, kOptValTypeNumber)) { } else if (option_has_type(opt_idx, kOptValTypeNumber)) {
if (opt->indir == PV_SCROLL) { if (opt->indir == PV_SCROLL) {
win_comp_scroll(curwin); win_comp_scroll(curwin);
} else { } else {
OptInt def_val = (OptInt)(intptr_t)opt->def_val; OptInt def_val = opt->def_val.number;
if ((OptInt *)varp == &curwin->w_p_so if ((OptInt *)varp == &curwin->w_p_so
|| (OptInt *)varp == &curwin->w_p_siso) { || (OptInt *)varp == &curwin->w_p_siso) {
// 'scrolloff' and 'sidescrolloff' local values have a // 'scrolloff' and 'sidescrolloff' local values have a
@ -465,7 +460,7 @@ static void set_option_default(const OptIndex opt_idx, int opt_flags)
} }
} }
} else { // boolean } else { // boolean
*(int *)varp = (int)(intptr_t)opt->def_val; *(int *)varp = opt->def_val.boolean;
#ifdef UNIX #ifdef UNIX
// 'modeline' defaults to off for root // 'modeline' defaults to off for root
if (opt->indir == PV_ML && getuid() == ROOT_UID) { if (opt->indir == PV_ML && getuid() == ROOT_UID) {
@ -521,10 +516,10 @@ static void set_string_default(OptIndex opt_idx, char *val, bool allocated)
vimoption_T *opt = &options[opt_idx]; vimoption_T *opt = &options[opt_idx];
if (opt->flags & P_DEF_ALLOCED) { if (opt->flags & P_DEF_ALLOCED) {
xfree(opt->def_val); xfree(opt->def_val.string);
} }
opt->def_val = allocated ? val : xstrdup(val); opt->def_val.string = allocated ? val : xstrdup(val);
opt->flags |= P_DEF_ALLOCED; opt->flags |= P_DEF_ALLOCED;
} }
@ -564,7 +559,7 @@ static char *find_dup_item(char *origval, const char *newval, uint32_t flags)
void set_number_default(OptIndex opt_idx, OptInt val) void set_number_default(OptIndex opt_idx, OptInt val)
{ {
if (opt_idx != kOptInvalid) { if (opt_idx != kOptInvalid) {
options[opt_idx].def_val = (void *)(intptr_t)val; options[opt_idx].def_val.number = val;
} }
} }
@ -639,11 +634,11 @@ void set_init_3(void)
|| path_fnamecmp(p, "tcsh") == 0) { || path_fnamecmp(p, "tcsh") == 0) {
if (do_sp) { if (do_sp) {
p_sp = "|& tee"; p_sp = "|& tee";
options[kOptShellpipe].def_val = p_sp; options[kOptShellpipe].def_val.string = p_sp;
} }
if (do_srr) { if (do_srr) {
p_srr = ">&"; p_srr = ">&";
options[kOptShellredir].def_val = p_srr; options[kOptShellredir].def_val.string = p_srr;
} }
} else if (path_fnamecmp(p, "sh") == 0 } else if (path_fnamecmp(p, "sh") == 0
|| path_fnamecmp(p, "ksh") == 0 || path_fnamecmp(p, "ksh") == 0
@ -658,11 +653,11 @@ void set_init_3(void)
// Always use POSIX shell style redirection if we reach this // Always use POSIX shell style redirection if we reach this
if (do_sp) { if (do_sp) {
p_sp = "2>&1| tee"; p_sp = "2>&1| tee";
options[kOptShellpipe].def_val = p_sp; options[kOptShellpipe].def_val.string = p_sp;
} }
if (do_srr) { if (do_srr) {
p_srr = ">%s 2>&1"; p_srr = ">%s 2>&1";
options[kOptShellredir].def_val = p_srr; options[kOptShellredir].def_val.string = p_srr;
} }
} }
xfree(p); xfree(p);
@ -724,12 +719,12 @@ void set_title_defaults(void)
// icon name. Saves a bit of time, because the X11 display server does // icon name. Saves a bit of time, because the X11 display server does
// not need to be contacted. // not need to be contacted.
if (!(options[kOptTitle].flags & P_WAS_SET)) { if (!(options[kOptTitle].flags & P_WAS_SET)) {
options[kOptTitle].def_val = 0; options[kOptTitle].def_val.boolean = false;
p_title = 0; p_title = false;
} }
if (!(options[kOptIcon].flags & P_WAS_SET)) { if (!(options[kOptIcon].flags & P_WAS_SET)) {
options[kOptIcon].def_val = 0; options[kOptIcon].def_val.boolean = false;
p_icon = 0; p_icon = false;
} }
} }
@ -751,7 +746,7 @@ void ex_set(exarg_T *eap)
/// Get the default value for a string option. /// Get the default value for a string option.
static char *stropt_get_default_val(OptIndex opt_idx, uint64_t flags) static char *stropt_get_default_val(OptIndex opt_idx, uint64_t flags)
{ {
char *newval = options[opt_idx].def_val; char *newval = options[opt_idx].def_val.string;
// expand environment variables and ~ since the default value was // expand environment variables and ~ since the default value was
// already expanded, only required when an environment variable was set // already expanded, only required when an environment variable was set
// later // later
@ -1178,7 +1173,7 @@ static OptVal get_option_newval(OptIndex opt_idx, int opt_flags, set_prefix_T pr
break; break;
} }
} else if (nextchar == '&') { } else if (nextchar == '&') {
newval_bool = TRISTATE_FROM_INT((int)(intptr_t)options[opt_idx].def_val); newval_bool = TRISTATE_FROM_INT(options[opt_idx].def_val.boolean);
} else if (nextchar == '<') { } else if (nextchar == '<') {
// For 'autoread', kNone means to use global value. // For 'autoread', kNone means to use global value.
if ((int *)varp == &curbuf->b_p_ar && opt_flags == OPT_LOCAL) { if ((int *)varp == &curbuf->b_p_ar && opt_flags == OPT_LOCAL) {
@ -1212,7 +1207,7 @@ static OptVal get_option_newval(OptIndex opt_idx, int opt_flags, set_prefix_T pr
// other error // other error
arg++; arg++;
if (nextchar == '&') { if (nextchar == '&') {
newval_num = (OptInt)(intptr_t)options[opt_idx].def_val; newval_num = options[opt_idx].def_val.number;
} else if (nextchar == '<') { } else if (nextchar == '<') {
if ((OptInt *)varp == &curbuf->b_p_ul && opt_flags == OPT_LOCAL) { if ((OptInt *)varp == &curbuf->b_p_ul && opt_flags == OPT_LOCAL) {
// for 'undolevels' NO_LOCAL_UNDOLEVEL means using the global newval_num // for 'undolevels' NO_LOCAL_UNDOLEVEL means using the global newval_num
@ -5311,7 +5306,7 @@ void reset_modifiable(void)
{ {
curbuf->b_p_ma = false; curbuf->b_p_ma = false;
p_ma = false; p_ma = false;
options[kOptModifiable].def_val = false; options[kOptModifiable].def_val.boolean = false;
} }
/// Set the global value for 'iminsert' to the local value. /// Set the global value for 'iminsert' to the local value.

View File

@ -60,7 +60,12 @@ typedef struct {
opt_expand_cb_T opt_expand_cb; opt_expand_cb_T opt_expand_cb;
// TODO(famiu): Use OptVal for def_val. // TODO(famiu): Use OptVal for def_val.
void *def_val; ///< default values for variable (neovim!!) union {
int boolean;
OptInt number;
char *string;
} def_val; ///< default value for variable
LastSet last_set; ///< script in which the option was last set LastSet last_set; ///< script in which the option was last set
} vimoption_T; } vimoption_T;

View File

@ -61,7 +61,7 @@ end
--- @return fun(): string --- @return fun(): string
local function macros(s) local function macros(s)
return function() return function()
return s return '.string=' .. s
end end
end end
@ -69,7 +69,7 @@ end
--- @return fun(): string --- @return fun(): string
local function imacros(s) local function imacros(s)
return function() return function()
return '(intptr_t)' .. s return '.number=' .. s
end end
end end
@ -1271,7 +1271,7 @@ return {
abbreviation = 'co', abbreviation = 'co',
cb = 'did_set_lines_or_columns', cb = 'did_set_lines_or_columns',
defaults = { defaults = {
if_true = macros('DFLT_COLS'), if_true = imacros('DFLT_COLS'),
doc = '80 or terminal width', doc = '80 or terminal width',
}, },
desc = [=[ desc = [=[
@ -4024,7 +4024,7 @@ return {
{ {
abbreviation = 'imi', abbreviation = 'imi',
cb = 'did_set_iminsert', cb = 'did_set_iminsert',
defaults = { if_true = macros('B_IMODE_NONE') }, defaults = { if_true = imacros('B_IMODE_NONE') },
desc = [=[ desc = [=[
Specifies whether :lmap or an Input Method (IM) is to be used in Specifies whether :lmap or an Input Method (IM) is to be used in
Insert mode. Valid values: Insert mode. Valid values:
@ -4050,7 +4050,7 @@ return {
}, },
{ {
abbreviation = 'ims', abbreviation = 'ims',
defaults = { if_true = macros('B_IMODE_USE_INSERT') }, defaults = { if_true = imacros('B_IMODE_USE_INSERT') },
desc = [=[ desc = [=[
Specifies whether :lmap or an Input Method (IM) is to be used when Specifies whether :lmap or an Input Method (IM) is to be used when
entering a search pattern. Valid values: entering a search pattern. Valid values:
@ -4747,7 +4747,7 @@ return {
{ {
cb = 'did_set_lines_or_columns', cb = 'did_set_lines_or_columns',
defaults = { defaults = {
if_true = macros('DFLT_ROWS'), if_true = imacros('DFLT_ROWS'),
doc = '24 or terminal height', doc = '24 or terminal height',
}, },
desc = [=[ desc = [=[