refactor(options): always allocate option values (#30917)

Instead of keeping `P_ALLOCED` and `P_DEF_ALLOCED` flags to check if an
option value is allocated, always allocate option values to simplify the
logic.

Ref: #25672
This commit is contained in:
Famiu Haque 2024-10-27 19:09:24 +06:00 committed by GitHub
parent 123c0b6b4e
commit b136a9ee4c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 47 additions and 114 deletions

View File

@ -67,7 +67,6 @@ local function get_flags(o)
end
end
for _, flag_desc in ipairs({
{ 'alloced' },
{ 'nodefault' },
{ 'no_mkrc' },
{ 'secure' },

View File

@ -713,12 +713,6 @@ EXTERN char *escape_chars INIT( = " \t\\\"|"); // need backslash in cmd line
EXTERN bool keep_help_flag INIT( = false); // doing :ta from help file
// When a string option is NULL (which only happens in out-of-memory situations), it is set to
// empty_string_option, to avoid having to check for NULL everywhere.
//
// TODO(famiu): Remove this when refcounted strings are used for string options.
EXTERN char *empty_string_option INIT( = "");
EXTERN bool redir_off INIT( = false); // no redirection for a moment
EXTERN FILE *redir_fd INIT( = NULL); // message redirection file
EXTERN int redir_reg INIT( = 0); // message redirection register

View File

@ -291,8 +291,7 @@ static void set_init_default_cdpath(void)
}
}
buf[j] = NUL;
options[kOptCdpath].def_val = CSTR_AS_OPTVAL(buf);
options[kOptCdpath].flags |= P_DEF_ALLOCED;
change_option_default(kOptCdpath, CSTR_AS_OPTVAL(buf));
xfree(cdpath);
}
@ -302,8 +301,6 @@ static void set_init_default_cdpath(void)
/// only happen for non-indirect options.
/// Also set the default to the expanded value, so ":set" does not list
/// them.
/// Don't set the P_ALLOCED flag, because we don't want to free the
/// default.
static void set_init_expand_env(void)
{
for (OptIndex opt_idx = 0; opt_idx < kOptIndexCount; opt_idx++) {
@ -318,14 +315,8 @@ static void set_init_expand_env(void)
p = option_expand(opt_idx, NULL);
}
if (p != NULL) {
set_option_varp(opt_idx, opt->var, CSTR_TO_OPTVAL(p), opt->flags & P_ALLOCED);
opt->flags |= P_ALLOCED;
if (opt->flags & P_DEF_ALLOCED) {
optval_free(opt->def_val);
}
opt->def_val = CSTR_TO_OPTVAL(p);
opt->flags |= P_DEF_ALLOCED;
set_option_varp(opt_idx, opt->var, CSTR_TO_OPTVAL(p), true);
change_option_default(opt_idx, CSTR_TO_OPTVAL(p));
}
}
}
@ -356,6 +347,9 @@ void set_init_1(bool clean_arg)
{
langmap_init();
// Allocate the default option values.
alloc_options_default();
set_init_default_shell();
set_init_default_backupskip();
set_init_default_cdpath();
@ -463,6 +457,25 @@ static OptVal get_option_default(const OptIndex opt_idx, int opt_flags)
}
}
/// Allocate the default values for all options by copying them from the stack.
/// This ensures that we don't need to always check if the option default is allocated or not.
static void alloc_options_default(void)
{
for (OptIndex opt_idx = 0; opt_idx < kOptIndexCount; opt_idx++) {
options[opt_idx].def_val = optval_copy(options[opt_idx].def_val);
}
}
/// Change the default value for an option.
///
/// @param opt_idx Option index in options[] table.
/// @param value New default value. Must be allocated.
static void change_option_default(const OptIndex opt_idx, OptVal value)
{
optval_free(options[opt_idx].def_val);
options[opt_idx].def_val = value;
}
/// Set an option to its default value.
/// This does not take care of side effects!
///
@ -512,17 +525,8 @@ static void set_options_default(int opt_flags)
static void set_string_default(OptIndex opt_idx, char *val, bool allocated)
FUNC_ATTR_NONNULL_ALL
{
if (opt_idx == kOptInvalid) {
return;
}
vimoption_T *opt = &options[opt_idx];
if (opt->flags & P_DEF_ALLOCED) {
optval_free(opt->def_val);
}
opt->def_val = CSTR_AS_OPTVAL(allocated ? val : xstrdup(val));
opt->flags |= P_DEF_ALLOCED;
assert(opt_idx != kOptInvalid);
change_option_default(opt_idx, CSTR_AS_OPTVAL(allocated ? val : xstrdup(val)));
}
/// For an option value that contains comma separated items, find "newval" in
@ -563,16 +567,14 @@ void free_all_options(void)
for (OptIndex opt_idx = 0; opt_idx < kOptIndexCount; opt_idx++) {
if (options[opt_idx].indir == PV_NONE) {
// global option: free value and default value.
if ((options[opt_idx].flags & P_ALLOCED) && options[opt_idx].var != NULL) {
if (options[opt_idx].var != NULL) {
optval_free(optval_from_varp(opt_idx, options[opt_idx].var));
}
if (options[opt_idx].flags & P_DEF_ALLOCED) {
optval_free(options[opt_idx].def_val);
}
} else if (options[opt_idx].var != VAR_WIN) {
// buffer-local option: free global value
optval_free(optval_from_varp(opt_idx, options[opt_idx].var));
}
optval_free(options[opt_idx].def_val);
}
free_operatorfunc_option();
free_tagfunc_option();
@ -600,7 +602,7 @@ void set_init_2(bool headless)
if (!option_was_set(kOptWindow)) {
p_window = Rows - 1;
}
options[kOptWindow].def_val = NUMBER_OPTVAL(Rows - 1);
change_option_default(kOptWindow, NUMBER_OPTVAL(Rows - 1));
}
/// Initialize the options, part three: After reading the .vimrc
@ -632,12 +634,12 @@ void set_init_3(void)
const OptVal sp =
is_csh ? STATIC_CSTR_AS_OPTVAL("|& tee") : STATIC_CSTR_AS_OPTVAL("2>&1| tee");
set_option_direct(kOptShellpipe, sp, 0, SID_NONE);
options[kOptShellpipe].def_val = sp;
change_option_default(kOptShellpipe, optval_copy(sp));
}
if (do_srr) {
const OptVal srr = is_csh ? STATIC_CSTR_AS_OPTVAL(">&") : STATIC_CSTR_AS_OPTVAL(">%s 2>&1");
set_option_direct(kOptShellredir, srr, 0, SID_NONE);
options[kOptShellredir].def_val = srr;
change_option_default(kOptShellredir, optval_copy(srr));
}
}
xfree(p);
@ -670,9 +672,7 @@ void set_helplang_default(const char *lang)
return;
}
if (options[kOptHelplang].flags & P_ALLOCED) {
free_string_option(p_hlg);
}
p_hlg = xmemdupz(lang, lang_len);
// zh_CN becomes "cn", zh_TW becomes "tw".
if (STRNICMP(p_hlg, "zh_", 3) == 0 && lang_len >= 5) {
@ -684,7 +684,6 @@ void set_helplang_default(const char *lang)
p_hlg[1] = 'n';
}
p_hlg[2] = NUL;
options[kOptHelplang].flags |= P_ALLOCED;
}
/// 'title' and 'icon' only default to true if they have not been set or reset
@ -698,11 +697,11 @@ void set_title_defaults(void)
// icon name. Saves a bit of time, because the X11 display server does
// not need to be contacted.
if (!(options[kOptTitle].flags & P_WAS_SET)) {
options[kOptTitle].def_val = BOOLEAN_OPTVAL(false);
change_option_default(kOptTitle, BOOLEAN_OPTVAL(false));
p_title = 0;
}
if (!(options[kOptIcon].flags & P_WAS_SET)) {
options[kOptIcon].def_val = BOOLEAN_OPTVAL(false);
change_option_default(kOptIcon, BOOLEAN_OPTVAL(false));
p_icon = 0;
}
}
@ -3441,7 +3440,6 @@ static const char *did_set_option(OptIndex opt_idx, void *varp, OptVal old_value
vimoption_T *opt = &options[opt_idx];
const char *errmsg = NULL;
bool restore_chartab = false;
bool free_oldval = (opt->flags & P_ALLOCED);
bool value_changed = false;
bool value_checked = false;
@ -3517,12 +3515,7 @@ static const char *did_set_option(OptIndex opt_idx, void *varp, OptVal old_value
set_option_sctx(opt_idx, opt_flags, script_ctx);
}
// Free options that are in allocated memory.
// Use "free_oldval", because recursiveness may change the flags (esp. init_highlight()).
if (free_oldval) {
optval_free(old_value);
}
opt->flags |= P_ALLOCED;
const bool scope_both = (opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0;
@ -5398,7 +5391,7 @@ void reset_modifiable(void)
{
curbuf->b_p_ma = false;
p_ma = false;
options[kOptModifiable].def_val = BOOLEAN_OPTVAL(false);
change_option_default(kOptModifiable, BOOLEAN_OPTVAL(false));
}
/// Set the global value for 'iminsert' to the local value.

View File

@ -8,16 +8,12 @@
// option_vars.h: definition of global variables for settable options
// Option Flags
#define P_ALLOCED 0x01U ///< the option is in allocated memory,
///< must use free_string_option() when
///< assigning new value. Not set if default is
///< the same.
// #define P_ALLOCED 0x01U ///< Not used
#define P_EXPAND 0x02U ///< environment expansion. NOTE: P_EXPAND can
///< never be used for local or hidden options
#define P_NO_DEF_EXP 0x04U ///< do not expand default value
#define P_NODEFAULT 0x08U ///< don't set to default value
#define P_DEF_ALLOCED 0x10U ///< default value is in allocated memory, must
///< use free() when assigning new value
// #define P_DEF_ALLOCED 0x10U ///< Not used
#define P_WAS_SET 0x20U ///< option has been set/reset
#define P_NO_MKRC 0x40U ///< don't include in :mkvimrc output
@ -348,6 +344,12 @@ enum {
#define LISPWORD_VALUE \
"defun,define,defmacro,set!,lambda,if,case,let,flet,let*,letrec,do,do*,define-syntax,let-syntax,letrec-syntax,destructuring-bind,defpackage,defparameter,defstruct,deftype,defvar,do-all-symbols,do-external-symbols,do-symbols,dolist,dotimes,ecase,etypecase,eval-when,labels,macrolet,multiple-value-bind,multiple-value-call,multiple-value-prog1,multiple-value-setq,prog1,progv,typecase,unless,unwind-protect,when,with-input-from-string,with-open-file,with-open-stream,with-output-to-string,with-package-iterator,define-condition,handler-bind,handler-case,restart-bind,restart-case,with-simple-restart,store-value,use-value,muffle-warning,abort,continue,with-slots,with-slots*,with-accessors,with-accessors*,defclass,defmethod,print-unreadable-object"
// When a string option is NULL, it is set to empty_string_option,
// to avoid having to check for NULL everywhere.
//
// TODO(famiu): Remove this when refcounted strings are used for string options.
EXTERN char empty_string_option[] INIT( = "");
// The following are actual variables for the options
EXTERN char *p_ambw; ///< 'ambiwidth'
@ -770,7 +772,7 @@ EXTERN unsigned ve_flags;
#define VE_NONEU 32U // "NONE"
EXTERN OptInt p_verbose; ///< 'verbose'
#ifdef IN_OPTION_C
char *p_vfile = ""; ///< used before options are initialized
char *p_vfile = empty_string_option; ///< used before options are initialized
#else
extern char *p_vfile; ///< 'verbosefile'
#endif

View File

@ -735,7 +735,6 @@ return {
},
{
abbreviation = 'briopt',
alloced = true,
cb = 'did_set_breakindentopt',
defaults = { if_true = '' },
deny_duplicates = true,
@ -798,7 +797,6 @@ return {
},
{
abbreviation = 'bh',
alloced = true,
cb = 'did_set_bufhidden',
defaults = { if_true = '' },
desc = [=[
@ -851,7 +849,6 @@ return {
},
{
abbreviation = 'bt',
alloced = true,
cb = 'did_set_buftype',
defaults = { if_true = '' },
desc = [=[
@ -1103,7 +1100,6 @@ return {
},
{
abbreviation = 'cink',
alloced = true,
defaults = { if_true = '0{,0},0),0],:,0#,!^F,o,O,e' },
deny_duplicates = true,
desc = [=[
@ -1122,7 +1118,6 @@ return {
},
{
abbreviation = 'cino',
alloced = true,
cb = 'did_set_cinoptions',
defaults = { if_true = '' },
deny_duplicates = true,
@ -1140,7 +1135,6 @@ return {
},
{
abbreviation = 'cinsd',
alloced = true,
defaults = { if_true = 'public,protected,private' },
deny_duplicates = true,
desc = [=[
@ -1159,7 +1153,6 @@ return {
},
{
abbreviation = 'cinw',
alloced = true,
defaults = { if_true = 'if,else,while,do,for,switch' },
deny_duplicates = true,
desc = [=[
@ -1309,7 +1302,6 @@ return {
},
{
abbreviation = 'com',
alloced = true,
cb = 'did_set_comments',
defaults = { if_true = 's1:/*,mb:*,ex:*/,://,b:#,:%,:XCOMM,n:>,fb:-,fb:•' },
deny_duplicates = true,
@ -1328,7 +1320,6 @@ return {
},
{
abbreviation = 'cms',
alloced = true,
cb = 'did_set_commentstring',
defaults = { if_true = '' },
desc = [=[
@ -1354,7 +1345,6 @@ return {
},
{
abbreviation = 'cpt',
alloced = true,
cb = 'did_set_complete',
defaults = { if_true = '.,w,b,u,t' },
deny_duplicates = true,
@ -1403,7 +1393,6 @@ return {
},
{
abbreviation = 'cfu',
alloced = true,
cb = 'did_set_completefunc',
defaults = { if_true = '' },
desc = [=[
@ -1525,7 +1514,6 @@ return {
},
{
abbreviation = 'cocu',
alloced = true,
cb = 'did_set_concealcursor',
defaults = { if_true = '' },
desc = [=[
@ -1968,7 +1956,6 @@ return {
},
{
abbreviation = 'def',
alloced = true,
defaults = { if_true = '' },
desc = [=[
Pattern to be used to find a macro definition. It is a search
@ -2088,7 +2075,6 @@ return {
},
{
abbreviation = 'dip',
alloced = true,
cb = 'did_set_diffopt',
defaults = { if_true = 'internal,filler,closeoff' },
deny_duplicates = true,
@ -2583,7 +2569,6 @@ return {
},
{
abbreviation = 'fenc',
alloced = true,
cb = 'did_set_encoding',
defaults = { if_true = '' },
desc = [=[
@ -2697,7 +2682,6 @@ return {
},
{
abbreviation = 'ff',
alloced = true,
cb = 'did_set_fileformat',
defaults = {
if_true = macros('DFLT_FF', 'string'),
@ -2813,7 +2797,6 @@ return {
},
{
abbreviation = 'ft',
alloced = true,
cb = 'did_set_filetype_or_syntax',
defaults = { if_true = '' },
desc = [=[
@ -2849,7 +2832,6 @@ return {
},
{
abbreviation = 'fcs',
alloced = true,
cb = 'did_set_chars_option',
defaults = { if_true = '' },
deny_duplicates = true,
@ -2964,7 +2946,6 @@ return {
},
{
abbreviation = 'fdc',
alloced = true,
cb = 'did_set_foldcolumn',
defaults = { if_true = '0' },
desc = [=[
@ -3003,7 +2984,6 @@ return {
},
{
abbreviation = 'fde',
alloced = true,
cb = 'did_set_foldexpr',
defaults = { if_true = '0' },
desc = [=[
@ -3029,7 +3009,6 @@ return {
},
{
abbreviation = 'fdi',
alloced = true,
cb = 'did_set_foldignore',
defaults = { if_true = '#' },
desc = [=[
@ -3084,7 +3063,6 @@ return {
},
{
abbreviation = 'fmr',
alloced = true,
cb = 'did_set_foldmarker',
defaults = { if_true = '{{{,}}}' },
deny_duplicates = true,
@ -3104,7 +3082,6 @@ return {
},
{
abbreviation = 'fdm',
alloced = true,
cb = 'did_set_foldmethod',
defaults = { if_true = 'manual' },
desc = [=[
@ -3205,7 +3182,6 @@ return {
},
{
abbreviation = 'fdt',
alloced = true,
cb = 'did_set_optexpr',
defaults = { if_true = 'foldtext()' },
desc = [=[
@ -3233,7 +3209,6 @@ return {
},
{
abbreviation = 'fex',
alloced = true,
cb = 'did_set_optexpr',
defaults = { if_true = '' },
desc = [=[
@ -3287,7 +3262,6 @@ return {
},
{
abbreviation = 'flp',
alloced = true,
defaults = { if_true = '^\\s*\\d\\+[\\]:.)}\\t ]\\s*' },
desc = [=[
A pattern that is used to recognize a list header. This is used for
@ -3308,7 +3282,6 @@ return {
},
{
abbreviation = 'fo',
alloced = true,
cb = 'did_set_formatoptions',
defaults = { if_true = macros('DFLT_FO_VIM', 'string') },
desc = [=[
@ -4151,7 +4124,6 @@ return {
},
{
abbreviation = 'inc',
alloced = true,
defaults = { if_true = '' },
desc = [=[
Pattern to be used to find an include command. It is a search
@ -4173,7 +4145,6 @@ return {
},
{
abbreviation = 'inex',
alloced = true,
cb = 'did_set_optexpr',
defaults = { if_true = '' },
desc = [=[
@ -4258,7 +4229,6 @@ return {
},
{
abbreviation = 'inde',
alloced = true,
cb = 'did_set_optexpr',
defaults = { if_true = '' },
desc = [=[
@ -4312,7 +4282,6 @@ return {
},
{
abbreviation = 'indk',
alloced = true,
defaults = { if_true = '0{,0},0),0],:,0#,!^F,o,O,e' },
deny_duplicates = true,
desc = [=[
@ -4454,7 +4423,6 @@ return {
},
{
abbreviation = 'isk',
alloced = true,
cb = 'did_set_iskeyword',
defaults = { if_true = '@,48-57,_,192-255' },
deny_duplicates = true,
@ -4563,7 +4531,6 @@ return {
},
{
abbreviation = 'kmp',
alloced = true,
cb = 'did_set_keymap',
defaults = { if_true = '' },
desc = [=[
@ -4940,7 +4907,6 @@ return {
},
{
abbreviation = 'lcs',
alloced = true,
cb = 'did_set_chars_option',
defaults = { if_true = 'tab:> ,trail:-,nbsp:+' },
deny_duplicates = true,
@ -5162,7 +5128,6 @@ return {
},
{
abbreviation = 'mps',
alloced = true,
cb = 'did_set_matchpairs',
defaults = { if_true = '(:),{:},[:]' },
deny_duplicates = true,
@ -5730,7 +5695,6 @@ return {
},
{
abbreviation = 'nf',
alloced = true,
cb = 'did_set_nrformats',
defaults = { if_true = 'bin,hex' },
deny_duplicates = true,
@ -5841,7 +5805,6 @@ return {
},
{
abbreviation = 'ofu',
alloced = true,
cb = 'did_set_omnifunc',
defaults = { if_true = '' },
desc = [=[
@ -6225,7 +6188,6 @@ return {
},
{
abbreviation = 'qe',
alloced = true,
defaults = { if_true = '\\' },
desc = [=[
The characters that are used to escape quotes in a string. Used for
@ -6436,7 +6398,6 @@ return {
},
{
abbreviation = 'rlc',
alloced = true,
cb = 'did_set_rightleftcmd',
defaults = { if_true = 'search' },
desc = [=[
@ -6491,7 +6452,6 @@ return {
},
{
abbreviation = 'ruf',
alloced = true,
cb = 'did_set_rulerformat',
defaults = { if_true = '' },
desc = [=[
@ -7640,7 +7600,6 @@ return {
},
{
abbreviation = 'scl',
alloced = true,
cb = 'did_set_signcolumn',
defaults = { if_true = 'auto' },
desc = [=[
@ -7797,7 +7756,6 @@ return {
},
{
abbreviation = 'spc',
alloced = true,
cb = 'did_set_spellcapcheck',
defaults = { if_true = '[.?!]\\_[\\])\'"\\t ]\\+' },
desc = [=[
@ -7820,7 +7778,6 @@ return {
},
{
abbreviation = 'spf',
alloced = true,
cb = 'did_set_spellfile',
defaults = { if_true = '' },
deny_duplicates = true,
@ -7858,7 +7815,6 @@ return {
},
{
abbreviation = 'spl',
alloced = true,
cb = 'did_set_spelllang',
defaults = { if_true = 'en' },
deny_duplicates = true,
@ -8092,7 +8048,6 @@ return {
},
{
abbreviation = 'stc',
alloced = true,
cb = 'did_set_statuscolumn',
defaults = { if_true = '' },
desc = [=[
@ -8158,7 +8113,6 @@ return {
},
{
abbreviation = 'stl',
alloced = true,
cb = 'did_set_statusline',
defaults = { if_true = '' },
desc = [=[
@ -8407,7 +8361,6 @@ return {
},
{
abbreviation = 'sua',
alloced = true,
defaults = { if_true = '' },
deny_duplicates = true,
desc = [=[
@ -8516,7 +8469,6 @@ return {
},
{
abbreviation = 'syn',
alloced = true,
cb = 'did_set_filetype_or_syntax',
defaults = { if_true = '' },
desc = [=[
@ -9003,7 +8955,6 @@ return {
},
{
abbreviation = 'tsrfu',
alloced = true,
cb = 'did_set_thesaurusfunc',
defaults = { if_true = '' },
desc = [=[
@ -9863,7 +9814,6 @@ return {
},
{
abbreviation = 'wbr',
alloced = true,
cb = 'did_set_winbar',
defaults = { if_true = '' },
desc = [=[
@ -10006,7 +9956,6 @@ return {
},
{
abbreviation = 'winhl',
alloced = true,
cb = 'did_set_winhighlight',
defaults = { if_true = '' },
deny_duplicates = true,

View File

@ -250,7 +250,6 @@ void check_buf_options(buf_T *buf)
/// Free the string allocated for an option.
/// Checks for the string being empty_string_option. This may happen if we're out of memory,
/// xstrdup() returned NULL, which was replaced by empty_string_option by check_options().
/// Does NOT check for P_ALLOCED flag!
void free_string_option(char *p)
{
if (p != empty_string_option) {

View File

@ -7348,7 +7348,6 @@ void ex_helpgrep(exarg_T *eap)
bool updated = false;
// Make 'cpoptions' empty, the 'l' flag should not be used here.
char *const save_cpo = p_cpo;
const bool save_cpo_allocated = (get_option(kOptCpoptions)->flags & P_ALLOCED);
p_cpo = empty_string_option;
bool new_qi = false;
@ -7388,10 +7387,8 @@ void ex_helpgrep(exarg_T *eap)
if (*p_cpo == NUL) {
set_option_value_give_err(kOptCpoptions, CSTR_AS_OPTVAL(save_cpo), 0);
}
if (save_cpo_allocated) {
free_string_option(save_cpo);
}
}
if (updated) {
// This may open a window and source scripts, do this after 'cpo' was