From adb3ec2026c08f6c36f47d4d4348ca6368543a90 Mon Sep 17 00:00:00 2001 From: oni-link Date: Tue, 24 Mar 2015 16:54:00 +0100 Subject: [PATCH 1/2] Update comments for expand_wildcards functions. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Be more specific in the description of mch_expand_wildcards(): This function will never free memory pointed to by its arguments. If OK is returned, *file will always point to allocated memory. *num_file is set to the number of pointers in *file. If FAIL is returned *file is set to NULL and *num_file to 0. If gen_expand_wildcards() returns FAIL, no memory allocation in this function needs to be undone. If expand_wildcards() returns FAIL, no memory allocation in this function needs to be undone. Helped-by: Eliseo Martínez Helped-by: Michael Reed --- src/nvim/ex_getln.c | 21 +++++++------- src/nvim/os_unix.c | 36 ++++++++++++----------- src/nvim/path.c | 69 ++++++++++++++++++++++++--------------------- 3 files changed, 68 insertions(+), 58 deletions(-) diff --git a/src/nvim/ex_getln.c b/src/nvim/ex_getln.c index d430509cfd..cd63b4fa90 100644 --- a/src/nvim/ex_getln.c +++ b/src/nvim/ex_getln.c @@ -3810,16 +3810,17 @@ void ExpandGeneric( reset_expand_highlight(); } -/* - * Complete a shell command. - */ -static void -expand_shellcmd ( - char_u *filepat, /* pattern to match with command names */ - int *num_file, /* return: number of matches */ - char_u ***file, /* return: array with matches */ - int flagsarg /* EW_ flags */ -) +/// Complete a shell command. +/// +/// @param filepat is a pattern to match with command names. +/// @param[out] num_file is pointer to number of matches. +/// @param[out] file is pointer to array of pointers to matches. +/// *file will either be set to NULL or point to +/// allocated memory. +/// @param flagsarg is a combination of EW_* flags. +static void expand_shellcmd(char_u *filepat, int *num_file, char_u ***file, + int flagsarg) + FUNC_ATTR_NONNULL_ALL { char_u *pat; int i; diff --git a/src/nvim/os_unix.c b/src/nvim/os_unix.c index 70cafc62ca..26a4cdb083 100644 --- a/src/nvim/os_unix.c +++ b/src/nvim/os_unix.c @@ -189,19 +189,6 @@ void mch_exit(int r) exit(r); } -/* - * mch_expand_wildcards() - this code does wild-card pattern matching using - * the shell - * - * return OK for success, FAIL for error (you may lose some memory) and put - * an error message in *file. - * - * num_pat is number of input patterns - * pat is array of pointers to input patterns - * num_file is pointer to number of matched file names - * file is pointer to array of pointers to matched file names - */ - #ifndef SEEK_SET # define SEEK_SET 0 #endif @@ -211,10 +198,27 @@ void mch_exit(int r) #define SHELL_SPECIAL (char_u *)"\t \"&'$;<>()\\|" +/// Does wildcard pattern matching using the shell. +/// +/// @param num_pat is the number of input patterns. +/// @param pat is an array of pointers to input patterns. +/// @param[out] num_file is pointer to number of matched file names. +/// Set to the number of pointers in *file. +/// @param[out] file is pointer to array of pointers to matched file names. +/// Memory pointed to by the initial value of *file will +/// not be freed. +/// Set to NULL if FAIL is returned. Otherwise points to +/// allocated memory. +/// @param flags is a combination of EW_* flags used in +/// expand_wildcards(). +/// If matching fails but EW_NOTFOUND is set in flags or +/// there are no wildcards, the patterns from pat are +/// copied into *file. +/// +/// @returns OK for success or FAIL for error. int mch_expand_wildcards(int num_pat, char_u **pat, int *num_file, - char_u ***file, - int flags /* EW_* flags */ - ) + char_u ***file, int flags) FUNC_ATTR_NONNULL_ARG(3) + FUNC_ATTR_NONNULL_ARG(4) { int i; size_t len; diff --git a/src/nvim/path.c b/src/nvim/path.c index e83b4e44ed..9515205643 100644 --- a/src/nvim/path.c +++ b/src/nvim/path.c @@ -1029,25 +1029,26 @@ static int has_special_wildchar(char_u *p) } #endif -/* - * Generic wildcard expansion code. - * - * Characters in "pat" that should not be expanded must be preceded with a - * backslash. E.g., "/path\ with\ spaces/my\*star*" - * - * Return FAIL when no single file was found. In this case "num_file" is not - * set, and "file" may contain an error message. - * Return OK when some files found. "num_file" is set to the number of - * matches, "file" to the array of matches. Call FreeWild() later. - */ -int -gen_expand_wildcards ( - int num_pat, /* number of input patterns */ - char_u **pat, /* array of input patterns */ - int *num_file, /* resulting number of files */ - char_u ***file, /* array of resulting files */ - int flags /* EW_* flags */ -) +/// Generic wildcard expansion code. +/// +/// Characters in pat that should not be expanded must be preceded with a +/// backslash. E.g., "/path\ with\ spaces/my\*star*". +/// +/// @param num_pat is number of input patterns. +/// @param pat is an array of pointers to input patterns. +/// @param[out] num_file is pointer to number of matched file names. +/// @param[out] file is pointer to array of pointers to matched file names. +/// @param flags is a combination of EW_* flags used in +/// expand_wildcards(). +/// +/// @returns OK when some files were found. *num_file is set to the +/// number of matches, *file to the allocated array of +/// matches. Call FreeWild() later. +/// If FAIL is returned, *num_file and *file are either +/// unchanged or *num_file is set to 0 and *file is set +/// to NULL or points to "". +int gen_expand_wildcards(int num_pat, char_u **pat, int *num_file, + char_u ***file, int flags) { int i; garray_T ga; @@ -1863,19 +1864,23 @@ int expand_wildcards_eval(char_u **pat, int *num_file, char_u ***file, return ret; } -/* - * Expand wildcards. Calls gen_expand_wildcards() and removes files matching - * 'wildignore'. - * Returns OK or FAIL. When FAIL then "num_file" won't be set. - */ -int -expand_wildcards ( - int num_pat, /* number of input patterns */ - char_u **pat, /* array of input patterns */ - int *num_file, /* resulting number of files */ - char_u ***file, /* array of resulting files */ - int flags /* EW_DIR, etc. */ -) +/// Expand wildcards. Calls gen_expand_wildcards() and removes files matching +/// 'wildignore'. +/// +/// @param num_pat is number of input patterns. +/// @param pat is an array of pointers to input patterns. +/// @param[out] num_file is pointer to number of matched file names. +/// @param[out] file is pointer to array of pointers to matched file names. +/// @param flags is a combination of EW_* flags. +/// +/// @returns OK when some files were found. *num_file is set to the +/// number of matches, *file to the allocated array of +/// matches. +/// If FAIL is returned, *num_file and *file are either +/// unchanged or *num_file is set to 0 and *file is set to +/// NULL or points to "". +int expand_wildcards(int num_pat, char_u **pat, int *num_file, char_u ***file, + int flags) { int retval; int i, j; From 8ee6f90bf810a3f0286513308e6cccff7d185666 Mon Sep 17 00:00:00 2001 From: oni-link Date: Tue, 24 Mar 2015 19:46:39 +0100 Subject: [PATCH 2/2] Fix problem with coverity/105568 fix. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original fix 3db0a40d691c103a26ef3df74528f12d89b0fa61 does not work for more than one loop iteration, because memory allocated in the previous iteration could be reused in the current iteration. Because expand_wildcards() never reads the variables *num_file and *file before the first assignment to them, the initial values for these variables can be anything. So instead of calling expand_shellcmd() with *file = "" we set *file = NULL. That should help coverity see, that not a array-typed value is freed. Helped-by: Eliseo Martínez --- src/nvim/ex_getln.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/nvim/ex_getln.c b/src/nvim/ex_getln.c index cd63b4fa90..f0beef87e7 100644 --- a/src/nvim/ex_getln.c +++ b/src/nvim/ex_getln.c @@ -10,7 +10,6 @@ * ex_getln.c: Functions for entering and editing an Ex command line. */ -#include #include #include #include @@ -3629,6 +3628,7 @@ ExpandFromContext ( } if (xp->xp_context == EXPAND_SHELLCMD) { + *file = NULL; expand_shellcmd(pat, num_file, file, flags); return OK; } @@ -3876,10 +3876,8 @@ static void expand_shellcmd(char_u *filepat, int *num_file, char_u ***file, STRLCPY(buf + l, pat, MAXPATHL - l); /* Expand matches in one directory of $PATH. */ - char_u **prev_file = *file; ret = expand_wildcards(1, &buf, num_file, file, flags); if (ret == OK) { - assert(*file != prev_file); ga_grow(&ga, *num_file); { for (i = 0; i < *num_file; ++i) {