patch 9.1.0685: too many strlen() calls in usercmd.cv9.1.0685

Problem:  too many strlen() calls in usercmd.c
Solution: refactor code to reduce the number or strlen() calls
          (John Marriott)

closes: #15516

Signed-off-by: John Marriott <basilisk@internode.on.net>
Signed-off-by: Christian Brabandt <cb@256bit.org>

3 files changed, 304 insertions, 213 deletions

diff --git a/src/testdir/test_usercommands.vim b/src/testdir/test_usercommands.vim
index e161ee63fe..fe08b9585c 100644
--- a/src/testdir/test_usercommands.vim
+++ b/src/testdir/test_usercommands.vim
@@ -133,6 +133,10 @@ function Test_cmdmods()
       \ 'silent verbose aboveleft belowright botright tab topleft vertical',
       \ g:mods)
 
+  kee keep keepm keepma keepmar keepmarks keepa keepalt keepj keepjumps
+      \ keepp keeppatterns MyCmd
+  call assert_equal('keepalt keepjumps keepmarks keeppatterns', g:mods)
+
   let g:mods = ''
   command! -nargs=* MyQCmd let g:mods .= '<q-mods> '
 
diff --git a/src/usercmd.c b/src/usercmd.c
index 3f0781c4fc..585ced3e77 100644
--- a/src/usercmd.c
+++ b/src/usercmd.c
@@ -16,6 +16,7 @@
 typedef struct ucmd
 {
     char_u	*uc_name;	// The command name
+    size_t	uc_namelen;	// The length of the command name (excluding the NUL)
     long_u	uc_argt;	// The argument type
     char_u	*uc_rep;	// The command's replacement string
     long	uc_def;		// The default value for a range/count
@@ -39,94 +40,100 @@ static int ucmd_locked = 0;
 
 /*
  * List of names for completion for ":command" with the EXPAND_ flag.
- * Must be alphabetical for completion.
+ * Must be alphabetical on the 'value' field for completion and because
+ * it is used by bsearch()!
  */
-static struct
+static keyvalue_T command_complete_tab[] =
 {
-    int	    expand;
-    char    *name;
-} command_complete[] =
-{
-    {EXPAND_ARGLIST, "arglist"},
-    {EXPAND_AUGROUP, "augroup"},
-    {EXPAND_BEHAVE, "behave"},
-    {EXPAND_BUFFERS, "buffer"},
-    {EXPAND_COLORS, "color"},
-    {EXPAND_COMMANDS, "command"},
-    {EXPAND_COMPILER, "compiler"},
+    KEYVALUE_ENTRY(EXPAND_ARGLIST, "arglist"),
+    KEYVALUE_ENTRY(EXPAND_AUGROUP, "augroup"),
+    KEYVALUE_ENTRY(EXPAND_BEHAVE, "behave"),
+#if defined(FEAT_EVAL)
+    KEYVALUE_ENTRY(EXPAND_BREAKPOINT, "breakpoint"),
+#endif
+    KEYVALUE_ENTRY(EXPAND_BUFFERS, "buffer"),
+    KEYVALUE_ENTRY(EXPAND_COLORS, "color"),
+    KEYVALUE_ENTRY(EXPAND_COMMANDS, "command"),
+    KEYVALUE_ENTRY(EXPAND_COMPILER, "compiler"),
 #if defined(FEAT_CSCOPE)
-    {EXPAND_CSCOPE, "cscope"},
+    KEYVALUE_ENTRY(EXPAND_CSCOPE, "cscope"),
 #endif
 #if defined(FEAT_EVAL)
-    {EXPAND_USER_DEFINED, "custom"},
-    {EXPAND_USER_LIST, "customlist"},
+    KEYVALUE_ENTRY(EXPAND_USER_DEFINED, "custom"),
+    KEYVALUE_ENTRY(EXPAND_USER_LIST, "customlist"),
 #endif
-    {EXPAND_DIFF_BUFFERS, "diff_buffer"},
-    {EXPAND_DIRECTORIES, "dir"},
-    {EXPAND_ENV_VARS, "environment"},
-    {EXPAND_EVENTS, "event"},
-    {EXPAND_EXPRESSION, "expression"},
-    {EXPAND_FILES, "file"},
-    {EXPAND_FILES_IN_PATH, "file_in_path"},
-    {EXPAND_FILETYPE, "filetype"},
-    {EXPAND_FUNCTIONS, "function"},
-    {EXPAND_HELP, "help"},
-    {EXPAND_HIGHLIGHT, "highlight"},
-    {EXPAND_HISTORY, "history"},
+    KEYVALUE_ENTRY(EXPAND_DIFF_BUFFERS, "diff_buffer"),
+    KEYVALUE_ENTRY(EXPAND_DIRECTORIES, "dir"),
+    KEYVALUE_ENTRY(EXPAND_DIRS_IN_CDPATH, "dir_in_path"),
+    KEYVALUE_ENTRY(EXPAND_ENV_VARS, "environment"),
+    KEYVALUE_ENTRY(EXPAND_EVENTS, "event"),
+    KEYVALUE_ENTRY(EXPAND_EXPRESSION, "expression"),
+    KEYVALUE_ENTRY(EXPAND_FILES, "file"),
+    KEYVALUE_ENTRY(EXPAND_FILES_IN_PATH, "file_in_path"),
+    KEYVALUE_ENTRY(EXPAND_FILETYPE, "filetype"),
+    KEYVALUE_ENTRY(EXPAND_FUNCTIONS, "function"),
+    KEYVALUE_ENTRY(EXPAND_HELP, "help"),
+    KEYVALUE_ENTRY(EXPAND_HIGHLIGHT, "highlight"),
+    KEYVALUE_ENTRY(EXPAND_HISTORY, "history"),
 #if defined(FEAT_KEYMAP)
-    {EXPAND_KEYMAP, "keymap"},
+    KEYVALUE_ENTRY(EXPAND_KEYMAP, "keymap"),
 #endif
 #if defined(HAVE_LOCALE_H) || defined(X_LOCALE)
-    {EXPAND_LOCALES, "locale"},
+    KEYVALUE_ENTRY(EXPAND_LOCALES, "locale"),
 #endif
-    {EXPAND_MAPCLEAR, "mapclear"},
-    {EXPAND_MAPPINGS, "mapping"},
-    {EXPAND_MENUS, "menu"},
-    {EXPAND_MESSAGES, "messages"},
-    {EXPAND_OWNSYNTAX, "syntax"},
-#if defined(FEAT_PROFILE)
-    {EXPAND_SYNTIME, "syntime"},
+    KEYVALUE_ENTRY(EXPAND_MAPCLEAR, "mapclear"),
+    KEYVALUE_ENTRY(EXPAND_MAPPINGS, "mapping"),
+    KEYVALUE_ENTRY(EXPAND_MENUS, "menu"),
+    KEYVALUE_ENTRY(EXPAND_MESSAGES, "messages"),
+    KEYVALUE_ENTRY(EXPAND_SETTINGS, "option"),
+    KEYVALUE_ENTRY(EXPAND_PACKADD, "packadd"),
+    KEYVALUE_ENTRY(EXPAND_RUNTIME, "runtime"),
+#if defined(FEAT_EVAL)
+    KEYVALUE_ENTRY(EXPAND_SCRIPTNAMES, "scriptnames"),
 #endif
-    {EXPAND_SETTINGS, "option"},
-    {EXPAND_PACKADD, "packadd"},
-    {EXPAND_RUNTIME, "runtime"},
-    {EXPAND_SHELLCMD, "shellcmd"},
+    KEYVALUE_ENTRY(EXPAND_SHELLCMD, "shellcmd"),
 #if defined(FEAT_SIGNS)
-    {EXPAND_SIGN, "sign"},
+    KEYVALUE_ENTRY(EXPAND_SIGN, "sign"),
 #endif
-    {EXPAND_TAGS, "tag"},
-    {EXPAND_TAGS_LISTFILES, "tag_listfiles"},
-    {EXPAND_USER, "user"},
-    {EXPAND_USER_VARS, "var"},
-#if defined(FEAT_EVAL)
-    {EXPAND_BREAKPOINT, "breakpoint"},
-    {EXPAND_SCRIPTNAMES, "scriptnames"},
+    KEYVALUE_ENTRY(EXPAND_OWNSYNTAX, "syntax"),
+#if defined(FEAT_PROFILE)
+    KEYVALUE_ENTRY(EXPAND_SYNTIME, "syntime"),
 #endif
-    {EXPAND_DIRS_IN_CDPATH, "dir_in_path"},
-    {0, NULL}
+    KEYVALUE_ENTRY(EXPAND_TAGS, "tag"),
+    KEYVALUE_ENTRY(EXPAND_TAGS_LISTFILES, "tag_listfiles"),
+    KEYVALUE_ENTRY(EXPAND_USER, "user"),
+    KEYVALUE_ENTRY(EXPAND_USER_VARS, "var")
 };
 
+typedef struct
+{
+    cmd_addr_T key;
+    char *fullname;
+    size_t fullnamelen;
+    char *shortname;
+    size_t shortnamelen;
+} addrtype_T;
+
 /*
  * List of names of address types.  Must be alphabetical for completion.
+ * Must be sorted by the 'fullname' field because it is used by bsearch()!
  */
-static struct
+#define ADDRTYPE_ENTRY(k, fn, sn) \
+	{(k), (fn), STRLEN_LITERAL(fn), (sn), STRLEN_LITERAL(sn)}
+static addrtype_T addr_type_complete_tab[] =
 {
-    cmd_addr_T	expand;
-    char	*name;
-    char	*shortname;
-} addr_type_complete[] =
-{
-    {ADDR_ARGUMENTS, "arguments", "arg"},
-    {ADDR_LINES, "lines", "line"},
-    {ADDR_LOADED_BUFFERS, "loaded_buffers", "load"},
-    {ADDR_TABS, "tabs", "tab"},
-    {ADDR_BUFFERS, "buffers", "buf"},
-    {ADDR_WINDOWS, "windows", "win"},
-    {ADDR_QUICKFIX, "quickfix", "qf"},
-    {ADDR_OTHER, "other", "?"},
-    {ADDR_NONE, NULL, NULL}
+    ADDRTYPE_ENTRY(ADDR_ARGUMENTS, "arguments", "arg"),
+    ADDRTYPE_ENTRY(ADDR_BUFFERS, "buffers", "buf"),
+    ADDRTYPE_ENTRY(ADDR_LINES, "lines", "line"),
+    ADDRTYPE_ENTRY(ADDR_LOADED_BUFFERS, "loaded_buffers", "load"),
+    ADDRTYPE_ENTRY(ADDR_OTHER, "other", "?"),
+    ADDRTYPE_ENTRY(ADDR_QUICKFIX, "quickfix", "qf"),
+    ADDRTYPE_ENTRY(ADDR_TABS, "tabs", "tab"),
+    ADDRTYPE_ENTRY(ADDR_WINDOWS, "windows", "win")
 };
 
+static int cmp_addr_type(const void *a, const void *b);
+
 /*
  * Search for a user command that matches "eap->cmd".
  * Return cmdidx in "eap->cmdidx", flags in "eap->argt", idx in "eap->useridx".
@@ -272,19 +279,16 @@ set_context_in_user_cmd(expand_T *xp, char_u *arg_in)
 	    {
 		xp->xp_context = EXPAND_USER_COMPLETE;
 		xp->xp_pattern = p + 1;
-		return NULL;
 	    }
 	    else if (STRNICMP(arg, "nargs", p - arg) == 0)
 	    {
 		xp->xp_context = EXPAND_USER_NARGS;
 		xp->xp_pattern = p + 1;
-		return NULL;
 	    }
 	    else if (STRNICMP(arg, "addr", p - arg) == 0)
 	    {
 		xp->xp_context = EXPAND_USER_ADDR_TYPE;
 		xp->xp_pattern = p + 1;
-		return NULL;
 	    }
 	    return NULL;
 	}
@@ -417,7 +421,9 @@ get_user_command_name(int idx, int cmdidx)
     char_u *
 get_user_cmd_addr_type(expand_T *xp UNUSED, int idx)
 {
-    return (char_u *)addr_type_complete[idx].name;
+    if (idx < 0 || idx >= (int)ARRAY_LENGTH(addr_type_complete_tab))
+	return NULL;
+    return (char_u *)addr_type_complete_tab[idx].fullname;
 }
 
 /*
@@ -432,7 +438,7 @@ get_user_cmd_flags(expand_T *xp UNUSED, int idx)
 	"count", "nargs", "range", "register", "keepscript"
     };
 
-    if (idx >= (int)ARRAY_LENGTH(user_cmd_flags))
+    if (idx < 0 || idx >= (int)ARRAY_LENGTH(user_cmd_flags))
 	return NULL;
     return (char_u *)user_cmd_flags[idx];
 }
@@ -445,7 +451,7 @@ get_user_cmd_nargs(expand_T *xp UNUSED, int idx)
 {
     static char *user_cmd_nargs[] = {"0", "1", "*", "?", "+"};
 
-    if (idx >= (int)ARRAY_LENGTH(user_cmd_nargs))
+    if (idx < 0 || idx >= (int)ARRAY_LENGTH(user_cmd_nargs))
 	return NULL;
     return (char_u *)user_cmd_nargs[idx];
 }
@@ -457,7 +463,24 @@ get_user_cmd_nargs(expand_T *xp UNUSED, int idx)
     char_u *
 get_user_cmd_complete(expand_T *xp UNUSED, int idx)
 {
-    return (char_u *)command_complete[idx].name;
+    if (idx < 0 || idx >= (int)ARRAY_LENGTH(command_complete_tab))
+	return NULL;
+    return (char_u *)command_complete_tab[idx].value;
+}
+
+/*
+ * Return the row in the command_complete_tab table that contains the given key.
+ */
+    static keyvalue_T *
+get_commandtype(int expand)
+{
+    int i;
+
+    for (i = 0; i < (int)ARRAY_LENGTH(command_complete_tab); ++i)
+	if (command_complete_tab[i].key == expand)
+	    return &command_complete_tab[i];
+
+    return NULL;
 }
 
 #ifdef FEAT_EVAL
@@ -467,13 +490,11 @@ get_user_cmd_complete(expand_T *xp UNUSED, int idx)
     char_u *
 cmdcomplete_type_to_str(int expand)
 {
-    int i;
+    keyvalue_T *kv;
 
-    for (i = 0; command_complete[i].expand != 0; i++)
-	if (command_complete[i].expand == expand)
-	    return (char_u *)command_complete[i].name;
+    kv = get_commandtype(expand);
 
-    return NULL;
+    return (kv == NULL) ? NULL : (char_u *)kv->value;
 }
 
 /*
@@ -483,18 +504,35 @@ cmdcomplete_type_to_str(int expand)
     int
 cmdcomplete_str_to_type(char_u *complete_str)
 {
-    int i;
+    keyvalue_T target;
+    keyvalue_T *entry;
+    static keyvalue_T *last_entry = NULL;	// cached result
 
     if (STRNCMP(complete_str, "custom,", 7) == 0)
 	return EXPAND_USER_DEFINED;
     if (STRNCMP(complete_str, "customlist,", 11) == 0)
 	return EXPAND_USER_LIST;
 
-    for (i = 0; command_complete[i].expand != 0; ++i)
-	if (STRCMP(complete_str, command_complete[i].name) == 0)
-	    return command_complete[i].expand;
+    target.key = 0;
+    target.value = (char *)complete_str;
+    target.length = 0;				// not used, see cmp_keyvalue_value()
+
+    if (last_entry != NULL && cmp_keyvalue_value(&target, last_entry) == 0)
+	entry = last_entry;
+    else
+    {
+	entry = (keyvalue_T *)bsearch(&target,
+	    &command_complete_tab,
+	    ARRAY_LENGTH(command_complete_tab),
+	    sizeof(command_complete_tab[0]),
+	    cmp_keyvalue_value);
+	if (entry == NULL)
+	    return EXPAND_NOTHING;
+
+	last_entry = entry;
+    }
 
-    return EXPAND_NOTHING;
+    return entry->key;
 }
 #endif
 
@@ -511,6 +549,7 @@ uc_list(char_u *name, size_t name_len)
     int		over;
     long	a;
     garray_T	*gap;
+    keyvalue_T	*entry;
 
     // don't allow for adding or removing user commands here
     ++ucmd_locked;
@@ -564,7 +603,7 @@ uc_list(char_u *name, size_t name_len)
 		msg_putchar(' ');
 
 	    msg_outtrans_attr(cmd->uc_name, HL_ATTR(HLF_D));
-	    len = (int)STRLEN(cmd->uc_name) + 4;
+	    len = (int)cmd->uc_namelen + 4;
 
 	    do {
 		msg_putchar(' ');
@@ -596,16 +635,14 @@ uc_list(char_u *name, size_t name_len)
 		if (a & EX_COUNT)
 		{
 		    // -count=N
-		    sprintf((char *)IObuff + len, "%ldc", cmd->uc_def);
-		    len += (int)STRLEN(IObuff + len);
+		    len += vim_snprintf((char *)IObuff + len, IOSIZE - len, "%ldc", cmd->uc_def);
 		}
 		else if (a & EX_DFLALL)
 		    IObuff[len++] = '%';
 		else if (cmd->uc_def >= 0)
 		{
 		    // -range=N
-		    sprintf((char *)IObuff + len, "%ld", cmd->uc_def);
-		    len += (int)STRLEN(IObuff + len);
+		    len += vim_snprintf((char *)IObuff + len, IOSIZE - len, "%ld", cmd->uc_def);
 		}
 		else
 		    IObuff[len++] = '.';
@@ -616,12 +653,12 @@ uc_list(char_u *name, size_t name_len)
 	    } while (len < 8 - over);
 
 	    // Address Type
-	    for (j = 0; addr_type_complete[j].expand != ADDR_NONE; ++j)
-		if (addr_type_complete[j].expand != ADDR_LINES
-			&& addr_type_complete[j].expand == cmd->uc_addr_type)
+	    for (j = 0; j < (int)ARRAY_LENGTH(addr_type_complete_tab); ++j)
+		if (addr_type_complete_tab[j].key != ADDR_LINES
+			&& addr_type_complete_tab[j].key == cmd->uc_addr_type)
 		{
-		    STRCPY(IObuff + len, addr_type_complete[j].shortname);
-		    len += (int)STRLEN(IObuff + len);
+		    STRCPY(IObuff + len, addr_type_complete_tab[j].shortname);
+		    len += (int)addr_type_complete_tab[j].shortnamelen;
 		    break;
 		}
 
@@ -630,28 +667,31 @@ uc_list(char_u *name, size_t name_len)
 	    } while (len < 13 - over);
 
 	    // Completion
-	    for (j = 0; command_complete[j].expand != 0; ++j)
-		if (command_complete[j].expand == cmd->uc_compl)
-		{
-		    STRCPY(IObuff + len, command_complete[j].name);
-		    len += (int)STRLEN(IObuff + len);
+	    entry = get_commandtype(cmd->uc_compl);
+	    if (entry != NULL)
+	    {
+		STRCPY(IObuff + len, entry->value);
+		len += entry->length;
 #ifdef FEAT_EVAL
-		    if (p_verbose > 0 && cmd->uc_compl_arg != NULL
-					    && STRLEN(cmd->uc_compl_arg) < 200)
+		if (p_verbose > 0 && cmd->uc_compl_arg != NULL)
+		{
+		    size_t uc_compl_arglen = STRLEN(cmd->uc_compl_arg);
+
+		    if (uc_compl_arglen < 200)
 		    {
-			IObuff[len] = ',';
-			STRCPY(IObuff + len + 1, cmd->uc_compl_arg);
-			len += (int)STRLEN(IObuff + len);
+			IObuff[len++] = ',';
+			STRCPY(IObuff + len, cmd->uc_compl_arg);
+			len += uc_compl_arglen;
 		    }
-#endif
-		    break;
 		}
+#endif
+	    }
 
 	    do {
 		IObuff[len++] = ' ';
 	    } while (len < 25 - over);
 
-	    IObuff[len] = '\0';
+	    IObuff[len] = NUL;
 	    msg_outtrans(IObuff);
 
 	    msg_outtrans_special(cmd->uc_rep, FALSE,
@@ -687,7 +727,7 @@ uc_fun_cmd(void)
 
     for (i = 0; fcmd[i]; ++i)
 	IObuff[i] = fcmd[i] - 0x40;
-    IObuff[i] = 0;
+    IObuff[i] = NUL;
     return (char *)IObuff;
 }
 
@@ -700,33 +740,52 @@ parse_addr_type_arg(
     int		vallen,
     cmd_addr_T	*addr_type_arg)
 {
-    int	    i, a, b;
+    addrtype_T target;
+    addrtype_T *entry;
+    static addrtype_T *last_entry;	// cached result
+
+    target.key = 0;
+    target.fullname = (char *)value;
+    target.fullnamelen = vallen;
 
-    for (i = 0; addr_type_complete[i].expand != ADDR_NONE; ++i)
+    if (last_entry != NULL && cmp_addr_type(&target, last_entry) == 0)
+	entry = last_entry;
+    else
     {
-	a = (int)STRLEN(addr_type_complete[i].name) == vallen;
-	b = STRNCMP(value, addr_type_complete[i].name, vallen) == 0;
-	if (a && b)
+	entry = (addrtype_T *)bsearch(&target,
+	    &addr_type_complete_tab,
+	    ARRAY_LENGTH(addr_type_complete_tab),
+	    sizeof(addr_type_complete_tab[0]),
+	    cmp_addr_type);
+	if (entry == NULL)
 	{
-	    *addr_type_arg = addr_type_complete[i].expand;
-	    break;
-	}
-    }
+	    int i;
+	    char_u	*err = value;
 
-    if (addr_type_complete[i].expand == ADDR_NONE)
-    {
-	char_u	*err = value;
+	    for (i = 0; err[i] != NUL && !VIM_ISWHITE(err[i]); i++)
+		;
+	    err[i] = NUL;
+	    semsg(_(e_invalid_address_type_value_str), err);
+	    return FAIL;
+	}
 
-	for (i = 0; err[i] != NUL && !VIM_ISWHITE(err[i]); i++)
-	    ;
-	err[i] = NUL;
-	semsg(_(e_invalid_address_type_value_str), err);
-	return FAIL;
+	last_entry = entry;
     }
 
+    *addr_type_arg = entry->key;
+
     return OK;
 }
 
+    static int
+cmp_addr_type(const void *a, const void *b)
+{
+    addrtype_T *at1 = (addrtype_T *)a;
+    addrtype_T *at2 = (addrtype_T *)b;
+
+    return STRNCMP(at1->fullname, at2->fullname, MAX(at1->fullnamelen, at2->fullnamelen));
+}
+
 /*
  * Parse a completion argument "value[vallen]".
  * The detected completion goes in "*complp", argument type in "*argt".
@@ -748,6 +807,9 @@ parse_compl_arg(
 # endif
     int		i;
     int		valend = vallen;
+    keyvalue_T	target;
+    keyvalue_T	*entry;
+    static keyvalue_T	*last_entry = NULL;	    // cached result
 
     // Look for any argument part - which is the part after any ','
     for (i = 0; i < vallen; ++i)
@@ -763,33 +825,40 @@ parse_compl_arg(
 	}
     }
 
-    for (i = 0; command_complete[i].expand != 0; ++i)
+    target.key = 0;
+    target.value = (char *)value;
+    target.length = valend;
+
+    if (last_entry != NULL && cmp_keyvalue_value_n(&target, last_entry) == 0)
+	entry = last_entry;
+    else
     {
-	if ((int)STRLEN(command_complete[i].name) == valend
-		&& STRNCMP(value, command_complete[i].name, valend) == 0)
+	entry = (keyvalue_T *)bsearch(&target,
+	    &command_complete_tab,
+	    ARRAY_LENGTH(command_complete_tab),
+	    sizeof(command_complete_tab[0]),
+	    cmp_keyvalue_value_n);
+	if (entry == NULL)
 	{
-	    *complp = command_complete[i].expand;
-	    if (command_complete[i].expand == EXPAND_BUFFERS)
-		*argt |= EX_BUFNAME;
-	    else if (command_complete[i].expand == EXPAND_DIRECTORIES
-		    || command_complete[i].expand == EXPAND_FILES)
-		*argt |= EX_XFILE;
-	    break;
+	    semsg(_(e_invalid_complete_value_str), value);
+	    return FAIL;
 	}
-    }
 
-    if (command_complete[i].expand == 0)
-    {
-	semsg(_(e_invalid_complete_value_str), value);
-	return FAIL;
+	last_entry = entry;
     }
 
+    *complp = entry->key;
+    if (*complp == EXPAND_BUFFERS)
+	*argt |= EX_BUFNAME;
+    else if (*complp == EXPAND_DIRECTORIES || *complp == EXPAND_FILES)
+	*argt |= EX_XFILE;
+
+    if (
 # if defined(FEAT_EVAL)
-    if (*complp != EXPAND_USER_DEFINED && *complp != EXPAND_USER_LIST
-							       && arg != NULL)
-# else
-    if (arg != NULL)
+	*complp != EXPAND_USER_DEFINED && *complp != EXPAND_USER_LIST
+								&&
 # endif
+								arg != NULL)
     {
 	emsg(_(e_completion_argument_only_allowed_for_custom_completion));
 	return FAIL;
@@ -806,6 +875,7 @@ parse_compl_arg(
     if (arg != NULL)
 	*compl_arg = vim_strnsave(arg, arglen);
 # endif
+
     return OK;
 }
 
@@ -1023,16 +1093,13 @@ uc_add_command(
     // Search for the command in the already defined commands.
     for (i = 0; i < gap->ga_len; ++i)
     {
-	size_t len;
-
 	cmd = USER_CMD_GA(gap, i);
-	len = STRLEN(cmd->uc_name);
 	cmp = STRNCMP(name, cmd->uc_name, name_len);
 	if (cmp == 0)
 	{
-	    if (name_len < len)
+	    if (name_len < cmd->uc_namelen)
 		cmp = -1;
-	    else if (name_len > len)
+	    else if (name_len > cmd->uc_namelen)
 		cmp = 1;
 	}
 
@@ -1078,6 +1145,7 @@ uc_add_command(
 	++gap->ga_len;
 
 	cmd->uc_name = p;
+	cmd->uc_namelen = name_len;
     }
 
     cmd->uc_rep = rep_buf;
@@ -1199,7 +1267,7 @@ ex_command(exarg_T *eap)
     p = skipwhite(end);
     if (!has_attr && ends_excmd2(eap->arg, p))
     {
-	uc_list(name, end - name);
+	uc_list(name, name_len);
     }
     else if (!ASCII_ISUPPER(*name))
     {
@@ -1282,6 +1350,7 @@ uc_clear(garray_T *gap)
     {
 	cmd = USER_CMD_GA(gap, i);
 	vim_free(cmd->uc_name);
+	cmd->uc_namelen = 0;
 	vim_free(cmd->uc_rep);
 # if defined(FEAT_EVAL)
 	vim_free(cmd->uc_compl_arg);
@@ -1439,30 +1508,31 @@ uc_split_args(char_u *arg, size_t *lenp)
 	}
     }
     *q++ = '"';
-    *q = 0;
+    *q = NUL;
 
     *lenp = len;
     return buf;
 }
 
     static size_t
-add_cmd_modifier(char_u *buf, char *mod_str, int *multi_mods)
+add_cmd_modifier(char_u *buf, size_t buflen, char *mod_str, size_t mod_strlen, int *multi_mods)
 {
-    size_t result;
-
-    result = STRLEN(mod_str);
-    if (*multi_mods)
-	result += 1;
     if (buf != NULL)
     {
 	if (*multi_mods)
-	    STRCAT(buf, " ");
-	STRCAT(buf, mod_str);
+	{
+	    STRCPY(buf + buflen, " ");	// the separating space
+	    ++buflen;
+	}
+	STRCPY(buf + buflen, mod_str);
     }
 
-    *multi_mods = 1;
+    if (*multi_mods)
+	++mod_strlen;			// +1 for the separating space
+    else
+	*multi_mods = 1;
 
-    return result;
+    return mod_strlen;
 }
 
 /*
@@ -1472,17 +1542,17 @@ add_cmd_modifier(char_u *buf, char *mod_str, int *multi_mods)
     size_t
 add_win_cmd_modifiers(char_u *buf, cmdmod_T *cmod, int *multi_mods)
 {
-    size_t result = 0;
+    size_t buflen = 0;
 
     // :aboveleft and :leftabove
     if (cmod->cmod_split & WSP_ABOVE)
-	result += add_cmd_modifier(buf, "aboveleft", multi_mods);
+	buflen += add_cmd_modifier(buf, buflen, "aboveleft", STRLEN_LITERAL("aboveleft"), multi_mods);
     // :belowright and :rightbelow
     if (cmod->cmod_split & WSP_BELOW)
-	result += add_cmd_modifier(buf, "belowright", multi_mods);
+	buflen += add_cmd_modifier(buf, buflen, "belowright", STRLEN_LITERAL("belowright"), multi_mods);
     // :botright
     if (cmod->cmod_split & WSP_BOT)
-	result += add_cmd_modifier(buf, "botright", multi_mods);
+	buflen += add_cmd_modifier(buf, buflen, "botright", STRLEN_LITERAL("botright"), multi_mods);
 
     // :tab
     if (cmod->cmod_tab > 0)
@@ -1493,27 +1563,29 @@ add_win_cmd_modifiers(char_u *buf, cmdmod_T *cmod, int *multi_mods)
 	{
 	    // For compatibility, don't add a tabpage number if it is the same
 	    // as the default number for :tab.
-	    result += add_cmd_modifier(buf, "tab", multi_mods);
+	    buflen += add_cmd_modifier(buf, buflen, "tab", STRLEN_LITERAL("tab"), multi_mods);
 	}
 	else
 	{
 	    char tab_buf[NUMBUFLEN + 3];
+	    size_t tab_buflen;
 
-	    sprintf(tab_buf, "%dtab", tabnr);
-	    result += add_cmd_modifier(buf, tab_buf, multi_mods);
+	    tab_buflen = vim_snprintf(tab_buf, sizeof(tab_buf), "%dtab", tabnr);
+	    buflen += add_cmd_modifier(buf, buflen, tab_buf, tab_buflen, multi_mods);
 	}
     }
 
     // :topleft
     if (cmod->cmod_split & WSP_TOP)
-	result += add_cmd_modifier(buf, "topleft", multi_mods);
+	buflen += add_cmd_modifier(buf, buflen, "topleft", STRLEN_LITERAL("topleft"), multi_mods);
     // :vertical
     if (cmod->cmod_split & WSP_VERT)
-	result += add_cmd_modifier(buf, "vertical", multi_mods);
+	buflen += add_cmd_modifier(buf, buflen, "vertical", STRLEN_LITERAL("vertical"), multi_mods);
     // :horizontal
     if (cmod->cmod_split & WSP_HOR)
-	result += add_cmd_modifier(buf, "horizontal", multi_mods);
-    return result;
+	buflen += add_cmd_modifier(buf, buflen, "horizontal", STRLEN_LITERAL("horizontal"), multi_mods);
+
+    return buflen;
 }
 
 /*
@@ -1523,78 +1595,92 @@ add_win_cmd_modifiers(char_u *buf, cmdmod_T *cmod, int *multi_mods)
     size_t
 produce_cmdmods(char_u *buf, cmdmod_T *cmod, int quote)
 {
-    size_t  result = 0;
+    size_t  buflen = 0;
     int	    multi_mods = 0;
     int	    i;
-    typedef struct {
-	int flag;
-	char *name;
-    } mod_entry_T;
-    static mod_entry_T mod_entries[] = {
+    static keyvalue_T mod_entry_tab[] =
+    {
 #ifdef FEAT_BROWSE_CMD
-	{CMOD_BROWSE, "browse"},
+	KEYVALUE_ENTRY(CMOD_BROWSE, "browse"),
 #endif
 #if defined(FEAT_GUI_DIALOG) || defined(FEAT_CON_DIALOG)
-	{CMOD_CONFIRM, "confirm"},
+	KEYVALUE_ENTRY(CMOD_CONFIRM, "confirm"),
 #endif
-	{CMOD_HIDE, "hide"},
-	{CMOD_KEEPALT, "keepalt"},
-	{CMOD_KEEPJUMPS, "keepjumps"},
-	{CMOD_KEEPMARKS, "keepmarks"},
-	{CMOD_KEEPPATTERNS, "keeppatterns"},
-	{CMOD_LOCKMARKS, "lockmarks"},
-	{CMOD_NOSWAPFILE, "noswapfile"},
-	{CMOD_UNSILENT, "unsilent"},
-	{CMOD_NOAUTOCMD, "noautocmd"},
+	KEYVALUE_ENTRY(CMOD_HIDE, "hide"),
+	KEYVALUE_ENTRY(CMOD_KEEPALT, "keepalt"),
+	KEYVALUE_ENTRY(CMOD_KEEPJUMPS, "keepjumps"),
+	KEYVALUE_ENTRY(CMOD_KEEPMARKS, "keepmarks"),
+	KEYVALUE_ENTRY(CMOD_KEEPPATTERNS, "keeppatterns"),
+	KEYVALUE_ENTRY(CMOD_LOCKMARKS, "lockmarks"),
+	KEYVALUE_ENTRY(CMOD_NOSWAPFILE, "noswapfile"),
+	KEYVALUE_ENTRY(CMOD_UNSILENT, "unsilent"),
+	KEYVALUE_ENTRY(CMOD_NOAUTOCMD, "noautocmd"),
 #ifdef HAVE_SANDBOX
-	{CMOD_SANDBOX, "sandbox"},
+	KEYVALUE_ENTRY(CMOD_SANDBOX, "sandbox"),
 #endif
-	{CMOD_LEGACY, "legacy"},
-	{0, NULL}
+	KEYVALUE_ENTRY(CMOD_LEGACY, "legacy")
     };
 
-    result = quote ? 2 : 0;
-    if (buf != NULL)
+    if (quote)
     {
-	if (quote)
-	    *buf++ = '"';
-	*buf = '\0';
+	++buflen;
+	if (buf != NULL)
+	{
+	    *buf = '"';
+	    *(buf + buflen) = NUL;
+	}
     }
+    else
+    if (buf != NULL)
+	*buf = NUL;
 
     // the modifiers that are simple flags
-    for (i = 0; mod_entries[i].name != NULL; ++i)
-	if (cmod->cmod_flags & mod_entries[i].flag)
-	    result += add_cmd_modifier(buf, mod_entries[i].name, &multi_mods);
+    for (i = 0; i < (int)ARRAY_LENGTH(mod_entry_tab); ++i)
+	if (cmod->cmod_flags & mod_entry_tab[i].key)
+	    buflen += add_cmd_modifier(buf, buflen, mod_entry_tab[i].value, mod_entry_tab[i].length, &multi_mods);
 
     // :silent
     if (cmod->cmod_flags & CMOD_SILENT)
-	result += add_cmd_modifier(buf,
-			(cmod->cmod_flags & CMOD_ERRSILENT) ? "silent!"
-						      : "silent", &multi_mods);
+    {
+	if (cmod->cmod_flags & CMOD_ERRSILENT)
+	    buflen += add_cmd_modifier(buf, buflen, "silent!", STRLEN_LITERAL("silent!"), &multi_mods);
+	else
+	    buflen += add_cmd_modifier(buf, buflen, "silent", STRLEN_LITERAL("silent"), &multi_mods);
+    }
+
     // :verbose
     if (cmod->cmod_verbose > 0)
     {
 	int verbose_value = cmod->cmod_verbose - 1;
 
 	if (verbose_value == 1)
-	    result += add_cmd_modifier(buf, "verbose", &multi_mods);
+	    buflen += add_cmd_modifier(buf, buflen, "verbose", STRLEN_LITERAL("verbose"), &multi_mods);
 	else
 	{
 	    char verbose_buf[NUMBUFLEN];
+	    size_t verbose_buflen;
 
-	    sprintf(verbose_buf, "%dverbose", verbose_value);
-	    result += add_cmd_modifier(buf, verbose_buf, &multi_mods);
+	    verbose_buflen = vim_snprintf(verbose_buf, sizeof(verbose_buf), "%dverbose", verbose_value);
+	    buflen += add_cmd_modifier(buf, buflen, verbose_buf, verbose_buflen, &multi_mods);
 	}
     }
+
     // flags from cmod->cmod_split
-    result += add_win_cmd_modifiers(buf, cmod, &multi_mods);
+    buflen += add_win_cmd_modifiers((buf == NULL) ? NULL : buf + buflen, cmod, &multi_mods);
 
-    if (quote && buf != NULL)
+    if (quote)
     {
-	buf += result - 2;
-	*buf = '"';
+	if (buf == NULL)
+	    ++buflen;
+	else
+	{
+	    *(buf + buflen) = '"';
+	    ++buflen;
+	    *(buf + buflen) = NUL;
+	}
     }
-    return result;
+
+    return buflen;
 }
 
 /*
@@ -1755,15 +1841,14 @@ uc_check_code(
     case ct_RANGE:
     case ct_COUNT:
     {
-	char num_buf[20];
+	char num_buf[NUMBUFLEN];
 	long num = (type == ct_LINE1) ? eap->line1 :
 		   (type == ct_LINE2) ? eap->line2 :
 		   (type == ct_RANGE) ? eap->addr_count :
 		   (eap->addr_count > 0) ? eap->line2 : cmd->uc_def;
 	size_t num_len;
 
-	sprintf(num_buf, "%ld", num);
-	num_len = STRLEN(num_buf);
+	num_len = vim_snprintf(num_buf, sizeof(num_buf), "%ld", num);
 	result = num_len;
 
 	if (quote)
diff --git a/src/version.c b/src/version.c
index 12dae15601..c0f0dff2c2 100644
--- a/src/version.c
+++ b/src/version.c
@@ -705,6 +705,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    685,
+/**/
     684,
 /**/
     683,