From 828a18722c2fb2d23560fd38ae182359e943d589 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Mon, 26 Jan 2015 21:28:41 +0100 Subject: coverity/13750: Negative array index read: FP. Problem : Negative array index read @ 909. Diagnostic : False positive. Rationale : Suggested error path assigns a negative value to idx at line 836 (`idx = find_command(ca.cmdchar);`). That's impossible, as `ca.cmdchar` is set to Ctrl_BSL just two lines above, so we know that value will be in the table. Resolution : Assert idx >= 0. --- src/nvim/normal.c | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/nvim/normal.c b/src/nvim/normal.c index f140922082..e147280723 100644 --- a/src/nvim/normal.c +++ b/src/nvim/normal.c @@ -834,6 +834,7 @@ getcount: ca.cmdchar = Ctrl_BSL; ca.nchar = c; idx = find_command(ca.cmdchar); + assert(idx >= 0); } } } -- cgit From ab86da74c4f81f492d493a42a1c3c26a273016a9 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Tue, 27 Jan 2015 15:22:36 +0100 Subject: coverity/68610: Out-of-bounds access: FP. Diagnostic : False positive. Rationale : Coverity thinks we are forgetting to add more char to hold NULL, but it's not taking into account that two chars from cntxformat will no be present in the result. In fact, we can even allocate one byte less than currently done. Resolution : Add explanatory comment and allocate one less byte. Marked as "Intentional" at coverity's database. --- src/nvim/if_cscope.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/nvim/if_cscope.c b/src/nvim/if_cscope.c index 843cbcf6f9..09f4ecf519 100644 --- a/src/nvim/if_cscope.c +++ b/src/nvim/if_cscope.c @@ -1646,7 +1646,6 @@ static void cs_print_tags_priv(char **matches, char **cntxts, int num_matches) char *fname, *lno, *extra, *tbuf; int i, idx, num; char *globalcntx = "GLOBAL"; - char *cntxformat = " <<%s>>"; char *context; char *cstag_msg = _("Cscope tag: %s"); @@ -1706,7 +1705,11 @@ static void cs_print_tags_priv(char **matches, char **cntxts, int num_matches) context = cntxts[idx]; else context = globalcntx; - newsize = strlen(context) + strlen(cntxformat); + + const char *cntxformat = " <<%s>>"; + // '%s' won't appear in result string, so: + // newsize = len(cntxformat) - 2 + len(context) + 1 (for NUL). + newsize = strlen(context) + strlen(cntxformat) - 1; if (bufsize < newsize) { buf = xrealloc(buf, newsize); -- cgit From 323f0488c24044971adf5180fed9f41d91a87b13 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Tue, 27 Jan 2015 15:51:52 +0100 Subject: coverity/15019: String not null terminated: FP. Problem : String not null terminated @ 1165. Diagnostic : False positive. Rationale : Code below terminates string (with NUL or '\n'). Resolution : Add explanatory comment, and assert termination. Mark as Intentional at coverity's database. --- src/nvim/os_unix.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/nvim/os_unix.c b/src/nvim/os_unix.c index f7b47f9569..15185aca49 100644 --- a/src/nvim/os_unix.c +++ b/src/nvim/os_unix.c @@ -13,6 +13,7 @@ * changed beyond recognition. */ +#include #include #include #include @@ -1162,6 +1163,8 @@ int mch_expand_wildcards(int num_pat, char_u **pat, int *num_file, len = ftell(fd); /* get size of temp file */ fseek(fd, 0L, SEEK_SET); buffer = xmalloc(len + 1); + // fread() doesn't terminate buffer with NUL; + // appropiate termination (not always NUL) is done below. i = fread((char *)buffer, 1, len, fd); fclose(fd); os_remove((char *)tempname); @@ -1174,8 +1177,6 @@ int mch_expand_wildcards(int num_pat, char_u **pat, int *num_file, } free(tempname); - - /* file names are separated with Space */ if (shell_style == STYLE_ECHO) { buffer[len] = '\n'; /* make sure the buffer ends in NL */ @@ -1235,6 +1236,8 @@ int mch_expand_wildcards(int num_pat, char_u **pat, int *num_file, if (len) ++i; /* count last entry */ } + assert(buffer[len] == NUL || buffer[len] == '\n'); + if (i == 0) { /* * Can happen when using /bin/sh and typing ":e $NO_SUCH_VAR^I". -- cgit From 4d0ef9a6b92953eb06937c3f74001909bd071c86 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Tue, 27 Jan 2015 17:02:57 +0100 Subject: coverity/13745: Argument cannot be negative: RI. Problem : Argument cannot be negative @ 1165. Diagnostic : Real issue. Rationale : len can be assigned a negative value @ 1162; len is passed as an unsigned argument @ 1165. Resolution : Refactor variable's types: - Use ftello instead of ftell to avoid using long. - Assert ftello result is safely convertible to size_t. - Introduce variable read_size to avoid using i (int). --- src/nvim/os_unix.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/nvim/os_unix.c b/src/nvim/os_unix.c index 15185aca49..d674db951f 100644 --- a/src/nvim/os_unix.c +++ b/src/nvim/os_unix.c @@ -1159,16 +1159,30 @@ int mch_expand_wildcards(int num_pat, char_u **pat, int *num_file, free(tempname); goto notfound; } - fseek(fd, 0L, SEEK_END); - len = ftell(fd); /* get size of temp file */ + int fseek_res = fseek(fd, 0L, SEEK_END); + if (fseek_res < 0) { + free(tempname); + fclose(fd); + return FAIL; + } + long long templen = ftell(fd); /* get size of temp file */ + if (templen < 0) { + free(tempname); + fclose(fd); + return FAIL; + } +#if SIZEOF_LONG_LONG > SIZEOF_SIZE_T + assert(templen <= (long long)SIZE_MAX); +#endif + len = (size_t)templen; fseek(fd, 0L, SEEK_SET); buffer = xmalloc(len + 1); // fread() doesn't terminate buffer with NUL; // appropiate termination (not always NUL) is done below. - i = fread((char *)buffer, 1, len, fd); + size_t readlen = fread((char *)buffer, 1, len, fd); fclose(fd); os_remove((char *)tempname); - if (i != (int)len) { + if (readlen != len) { /* unexpected read error */ EMSG2(_(e_notread), tempname); free(tempname); -- cgit From bb674e0fcdc19ee069ad5130e5978d24b90765d0 Mon Sep 17 00:00:00 2001 From: Eliseo Martínez Date: Tue, 27 Jan 2015 18:11:06 +0100 Subject: coverity/13810: Unitialized scalar variable: HI. Problem : Unitialized scalar variable @ 3239. Diagnostic : Harmless issue. Rationale : It's true pos.coladd is not initialized when calling searchit(). But that's no problem, as coladd is only set in that function. Resolution : Initialize variable to 0. --- src/nvim/ex_docmd.c | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) (limited to 'src') diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c index f6527db69b..9b48398d96 100644 --- a/src/nvim/ex_docmd.c +++ b/src/nvim/ex_docmd.c @@ -3219,27 +3219,15 @@ get_address ( } if (!skip) { - /* - * When search follows another address, start from - * there. - */ - if (lnum != MAXLNUM) - pos.lnum = lnum; - else - pos.lnum = curwin->w_cursor.lnum; - - /* - * Start the search just like for the above - * do_search(). - */ - if (*cmd != '?') - pos.col = MAXCOL; - else - pos.col = 0; + // When search follows another address, start from there. + pos.lnum = (lnum != MAXLNUM) ? lnum : curwin->w_cursor.lnum; + // Start the search just like for the above do_search(). + pos.col = (*cmd != '?') ? MAXCOL : 0; + pos.coladd = 0; if (searchit(curwin, curbuf, &pos, - *cmd == '?' ? BACKWARD : FORWARD, - (char_u *)"", 1L, SEARCH_MSG, - i, (linenr_T)0, NULL) != FAIL) + *cmd == '?' ? BACKWARD : FORWARD, + (char_u *)"", 1L, SEARCH_MSG, + i, (linenr_T)0, NULL) != FAIL) lnum = pos.lnum; else { cmd = NULL; -- cgit