patch 8.2.4474: memory allocation failures not tested in quickfix code

Problem:    Memory allocation failures not tested in quickfix code.
Solution:   Add alloc IDs and tests. (Yegappan Lakshmanan, closes #9848)

diff --git a/src/alloc.h b/src/alloc.h
index 8babde3bd7f1ff6ea0dca4f1f108437dfc7220ed..9260018994635edee1e4870444e26c69f9f14044 100644 (file)
--- a/src/alloc.h
+++ b/src/alloc.h
@@ -8,26 +8,37 @@
 
 /*
  * alloc.h: enumeration of alloc IDs.
+ * Used by test_alloc_fail() to test memory allocation failures.
  * Each entry must be on exactly one line, GetAllocId() depends on that.
  */
 typedef enum {
-       aid_none = 0,
-       aid_qf_dirname_start,
-       aid_qf_dirname_now,
-       aid_qf_namebuf,
-       aid_qf_module,
-       aid_qf_errmsg,
-       aid_qf_pattern,
-       aid_tagstack_items,
-       aid_tagstack_from,
-       aid_tagstack_details,
-       aid_sign_getdefined,
-       aid_sign_getplaced,
-       aid_sign_define_by_name,
-       aid_sign_getlist,
-       aid_sign_getplaced_dict,
-       aid_sign_getplaced_list,
-       aid_insert_sign,
-       aid_sign_getinfo,
-       aid_last
+    aid_none = 0,
+    aid_qf_dirname_start,
+    aid_qf_dirname_now,
+    aid_qf_namebuf,
+    aid_qf_module,
+    aid_qf_errmsg,
+    aid_qf_pattern,
+    aid_qf_efm_fmtstr,
+    aid_qf_efm_fmtpart,
+    aid_qf_title,
+    aid_qf_mef_name,
+    aid_qf_qfline,
+    aid_qf_qfinfo,
+    aid_qf_dirstack,
+    aid_qf_multiline_pfx,
+    aid_qf_makecmd,
+    aid_qf_linebuf,
+    aid_tagstack_items,
+    aid_tagstack_from,
+    aid_tagstack_details,
+    aid_sign_getdefined,
+    aid_sign_getplaced,
+    aid_sign_define_by_name,
+    aid_sign_getlist,
+    aid_sign_getplaced_dict,
+    aid_sign_getplaced_list,
+    aid_insert_sign,
+    aid_sign_getinfo,
+    aid_last
 } alloc_id_T;

diff --git a/src/quickfix.c b/src/quickfix.c
index f954a16bfd564095905c87e06e32cac5ba95ee10..10fc4e09f1d01ab6a57a75cc12253572b99526d4 100644 (file)
--- a/src/quickfix.c
+++ b/src/quickfix.c
@@ -547,13 +547,13 @@ parse_efm_option(char_u *efm)
 
     // Get some space to modify the format string into.
     sz = efm_regpat_bufsz(efm);
-    if ((fmtstr = alloc(sz)) == NULL)
+    if ((fmtstr = alloc_id(sz, aid_qf_efm_fmtstr)) == NULL)
        goto parse_efm_error;
 
     while (efm[0] != NUL)
     {
        // Allocate a new eformat structure and put it at the end of the list
-       fmt_ptr = ALLOC_CLEAR_ONE(efm_T);
+       fmt_ptr = ALLOC_CLEAR_ONE_ID(efm_T, aid_qf_efm_fmtpart);
        if (fmt_ptr == NULL)
            goto parse_efm_error;
        if (fmt_first == NULL)      // first one
@@ -628,7 +628,7 @@ qf_grow_linebuf(qfstate_T *state, int newsz)
     state->linelen = newsz > LINE_MAXLEN ? LINE_MAXLEN - 1 : newsz;
     if (state->growbuf == NULL)
     {
-       state->growbuf = alloc(state->linelen + 1);
+       state->growbuf = alloc_id(state->linelen + 1, aid_qf_linebuf);
        if (state->growbuf == NULL)
            return NULL;
        state->growbufsiz = state->linelen;
@@ -685,7 +685,7 @@ qf_get_next_str_line(qfstate_T *state)
 }
 
 /*
- * Get the next string from state->p_Li.
+ * Get the next string from the List item state->p_li.
  */
     static int
 qf_get_next_list_line(qfstate_T *state)
@@ -777,7 +777,7 @@ qf_get_next_file_line(qfstate_T *state)
        if (state->growbuf == NULL)
        {
            state->growbufsiz = 2 * (IOSIZE - 1);
-           state->growbuf = alloc(state->growbufsiz);
+           state->growbuf = alloc_id(state->growbufsiz, aid_qf_linebuf);
            if (state->growbuf == NULL)
                return QF_NOMEM;
        }
@@ -1382,8 +1382,9 @@ qf_parse_multiline_pfx(
        if (*fields->errmsg && !qfl->qf_multiignore)
        {
            len = (int)STRLEN(qfprev->qf_text);
-           if ((ptr = alloc(len + STRLEN(fields->errmsg) + 2))
-                   == NULL)
+           ptr = alloc_id(len + STRLEN(fields->errmsg) + 2,
+                                               aid_qf_multiline_pfx);
+           if (ptr == NULL)
                return QF_FAIL;
            STRCPY(ptr, qfprev->qf_text);
            vim_free(qfprev->qf_text);
@@ -1859,7 +1860,7 @@ qf_store_title(qf_list_T *qfl, char_u *title)
 
     if (title != NULL)
     {
-       char_u *p = alloc(STRLEN(title) + 2);
+       char_u *p = alloc_id(STRLEN(title) + 2, aid_qf_title);
 
        qfl->qf_title = p;
        if (p != NULL)
@@ -2109,7 +2110,7 @@ qf_add_entry(
     qfline_T   *qfp;
     qfline_T   **lastp;        // pointer to qf_last or NULL
 
-    if ((qfp = ALLOC_ONE(qfline_T)) == NULL)
+    if ((qfp = ALLOC_ONE_ID(qfline_T, aid_qf_qfline)) == NULL)
        return QF_FAIL;
     if (bufnum != 0)
     {
@@ -2189,7 +2190,7 @@ qf_alloc_stack(qfltype_T qfltype)
 {
     qf_info_T *qi;
 
-    qi = ALLOC_CLEAR_ONE(qf_info_T);
+    qi = ALLOC_CLEAR_ONE_ID(qf_info_T, aid_qf_qfinfo);
     if (qi != NULL)
     {
        qi->qf_refcount++;
@@ -2483,7 +2484,7 @@ qf_push_dir(char_u *dirbuf, struct dir_stack_T **stackptr, int is_file_stack)
     struct dir_stack_T  *ds_ptr;
 
     // allocate new stack element and hook it in
-    ds_new = ALLOC_ONE(struct dir_stack_T);
+    ds_new = ALLOC_ONE_ID(struct dir_stack_T, aid_qf_dirstack);
     if (ds_new == NULL)
        return NULL;
 
@@ -4945,7 +4946,7 @@ get_mef_name(void)
        else
            off += 19;
 
-       name = alloc(STRLEN(p_mef) + 30);
+       name = alloc_id(STRLEN(p_mef) + 30, aid_qf_mef_name);
        if (name == NULL)
            break;
        STRCPY(name, p_mef);
@@ -4976,7 +4977,7 @@ make_get_fullcmd(char_u *makecmd, char_u *fname)
     len = (unsigned)STRLEN(p_shq) * 2 + (unsigned)STRLEN(makecmd) + 1;
     if (*p_sp != NUL)
        len += (unsigned)STRLEN(p_sp) + (unsigned)STRLEN(fname) + 3;
-    cmd = alloc(len);
+    cmd = alloc_id(len, aid_qf_makecmd);
     if (cmd == NULL)
        return NULL;
     sprintf((char *)cmd, "%s%s%s", (char *)p_shq, (char *)makecmd,
@@ -5042,7 +5043,10 @@ ex_make(exarg_T *eap)
 
     cmd = make_get_fullcmd(eap->arg, fname);
     if (cmd == NULL)
+    {
+       vim_free(fname);
        return;
+    }
 
     // let the shell know if we are redirecting output or not
     do_shell(cmd, *p_sp != NUL ? SHELL_DOOUT : 0);

diff --git a/src/testdir/test_quickfix.vim b/src/testdir/test_quickfix.vim
index 5dd72177753433fa9a25cebcc167c0c62e2da9ee..42b819376752db886d8ec4d19c5c80b46dc34226 100644 (file)
--- a/src/testdir/test_quickfix.vim
+++ b/src/testdir/test_quickfix.vim
@@ -621,22 +621,147 @@ func Test_browse()
   call Xtest_browse('l')
 endfunc
 
-func Test_nomem()
+" Test for memory allocation failures
+func Xnomem_tests(cchar)
+  call s:setup_commands(a:cchar)
+
   call test_alloc_fail(GetAllocId('qf_dirname_start'), 0, 0)
-  call assert_fails('vimgrep vim runtest.vim', 'E342:')
+  call assert_fails('Xvimgrep vim runtest.vim', 'E342:')
 
-  call GetAllocId('qf_dirname_now')->test_alloc_fail(0, 0)
-  call assert_fails('vimgrep vim runtest.vim', 'E342:')
+  call test_alloc_fail(GetAllocId('qf_dirname_now'), 0, 0)
+  call assert_fails('Xvimgrep vim runtest.vim', 'E342:')
 
   call test_alloc_fail(GetAllocId('qf_namebuf'), 0, 0)
-  call assert_fails('cfile runtest.vim', 'E342:')
+  call assert_fails('Xfile runtest.vim', 'E342:')
 
   call test_alloc_fail(GetAllocId('qf_errmsg'), 0, 0)
-  call assert_fails('cfile runtest.vim', 'E342:')
+  call assert_fails('Xfile runtest.vim', 'E342:')
 
   call test_alloc_fail(GetAllocId('qf_pattern'), 0, 0)
-  call assert_fails('cfile runtest.vim', 'E342:')
+  call assert_fails('Xfile runtest.vim', 'E342:')
+
+  call test_alloc_fail(GetAllocId('qf_efm_fmtstr'), 0, 0)
+  set efm=%f
+  call assert_fails('Xexpr ["Xfile1"]', 'E342:')
+  set efm&
+
+  call test_alloc_fail(GetAllocId('qf_efm_fmtpart'), 0, 0)
+  set efm=%f:%l:%m,%f-%l-%m
+  call assert_fails('Xaddexpr ["Xfile2", "Xfile3"]', 'E342:')
+  set efm&
+
+  call test_alloc_fail(GetAllocId('qf_title'), 0, 0)
+  call assert_fails('Xexpr ""', 'E342:')
+  call assert_equal('', g:Xgetlist({'all': 1}).title)
+
+  call test_alloc_fail(GetAllocId('qf_mef_name'), 0, 0)
+  set makeef=Xtmp##.err
+  call assert_fails('Xgrep needle haystack', 'E342:')
+  set makeef&
+
+  call test_alloc_fail(GetAllocId('qf_qfline'), 0, 0)
+  call assert_fails('Xexpr "Xfile1:10:Line10"', 'E342:')
+
+  if a:cchar == 'l'
+    for id in ['qf_qfline', 'qf_qfinfo']
+      lgetexpr ["Xfile1:10:L10", "Xfile2:20:L20"]
+      call test_alloc_fail(GetAllocId(id), 0, 0)
+      call assert_fails('new', 'E342:')
+      call assert_equal(2, winnr('$'))
+      call assert_equal([], getloclist(0))
+      %bw!
+    endfor
+  endif
+
+  call test_alloc_fail(GetAllocId('qf_qfline'), 0, 0)
+  try
+    call assert_fails('Xvimgrep vim runtest.vim', 'E342:')
+  catch /^Vim:Interrupt$/
+  endtry
+
+  call test_alloc_fail(GetAllocId('qf_qfline'), 0, 0)
+  try
+    call assert_fails('Xvimgrep /vim/f runtest.vim', 'E342:')
+  catch /^Vim:Interrupt$/
+  endtry
+
+  let l = getqflist({"lines": ["Xfile1:10:L10"]})
+  call test_alloc_fail(GetAllocId('qf_qfline'), 0, 0)
+  call assert_fails('call g:Xsetlist(l.items)', 'E342:')
+
+  call test_alloc_fail(GetAllocId('qf_qfline'), 0, 0)
+  try
+    call assert_fails('Xhelpgrep quickfix', 'E342:')
+  catch /^Vim:Interrupt$/
+  endtry
+
+  call test_alloc_fail(GetAllocId('qf_qfinfo'), 0, 0)
+  call assert_fails('let l = g:Xgetlist({"lines": ["Xfile1:10:L10"]})', 'E342:')
+  call assert_equal(#{items: []}, l)
 
+  if a:cchar == 'l'
+    call setqflist([], 'f')
+    call setloclist(0, [], 'f')
+    call test_alloc_fail(GetAllocId('qf_qfinfo'), 0, 0)
+    call assert_fails('lhelpgrep quickfix', 'E342:')
+    call assert_equal([], getloclist(0))
+
+    call test_alloc_fail(GetAllocId('qf_qfinfo'), 0, 0)
+    call assert_fails('lvimgrep vim runtest.vim', 'E342:')
+
+    let l = getqflist({"lines": ["Xfile1:10:L10"]})
+    call test_alloc_fail(GetAllocId('qf_qfinfo'), 0, 0)
+    call assert_fails('call setloclist(0, l.items)', 'E342:')
+
+    call test_alloc_fail(GetAllocId('qf_qfinfo'), 0, 0)
+    call assert_fails('lbuffer', 'E342:')
+
+    call test_alloc_fail(GetAllocId('qf_qfinfo'), 0, 0)
+    call assert_fails('lexpr ["Xfile1:10:L10", "Xfile2:20:L20"]', 'E342:')
+
+    call test_alloc_fail(GetAllocId('qf_qfinfo'), 0, 0)
+    call assert_fails('lfile runtest.vim', 'E342:')
+  endif
+
+  call test_alloc_fail(GetAllocId('qf_dirstack'), 0, 0)
+  set efm=%DEntering\ dir\ %f,%f:%l:%m
+  call assert_fails('Xexpr ["Entering dir abc", "abc.txt:1:Hello world"]', 'E342:')
+  set efm&
+
+  call test_alloc_fail(GetAllocId('qf_dirstack'), 0, 0)
+  set efm=%+P[%f],(%l)%m
+  call assert_fails('Xexpr ["[runtest.vim]", "(1)Hello"]', 'E342:')
+  set efm&
+
+  call test_alloc_fail(GetAllocId('qf_multiline_pfx'), 0, 0)
+  set efm=%EError,%Cline\ %l,%Z%m
+  call assert_fails('Xexpr ["Error", "line 1", "msg"]', 'E342:')
+  set efm&
+
+  call test_alloc_fail(GetAllocId('qf_makecmd'), 0, 0)
+  call assert_fails('Xgrep vim runtest.vim', 'E342:')
+
+  call test_alloc_fail(GetAllocId('qf_linebuf'), 0, 0)
+  call assert_fails('Xexpr repeat("a", 8192)', 'E342:')
+
+  call test_alloc_fail(GetAllocId('qf_linebuf'), 0, 0)
+  call assert_fails('Xexpr [repeat("a", 8192)]', 'E342:')
+
+  new
+  call setline(1, repeat('a', 8192))
+  call test_alloc_fail(GetAllocId('qf_linebuf'), 0, 0)
+  call assert_fails('Xbuffer', 'E342:')
+  %bw!
+
+  call writefile([repeat('a', 8192)], 'Xtest')
+  call test_alloc_fail(GetAllocId('qf_linebuf'), 0, 0)
+  call assert_fails('Xfile Xtest', 'E342:')
+  call delete('Xtest')
+endfunc
+
+func Test_nomem()
+  call Xnomem_tests('c')
+  call Xnomem_tests('l')
 endfunc
 
 func s:test_xhelpgrep(cchar)

diff --git a/src/version.c b/src/version.c
index 7b5d66a84e4afbc1b2b3f465dc909986dad15376..c7c7402ea296419e89b8bc070064691f0c1ded18 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -754,6 +754,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    4474,
 /**/
     4473,
 /**/

diff --git a/src/vim.h b/src/vim.h
index 09303e98c34e12ae3904fdf2621d7e779191194c..816cfe89de98f49c783bc60952de420ff710f621 100644 (file)
--- a/src/vim.h
+++ b/src/vim.h
@@ -1608,8 +1608,10 @@ typedef UINT32_TYPEDEF UINT32_T;
 // Allocate memory for one type and cast the returned pointer to have the
 // compiler check the types.
 #define ALLOC_ONE(type)  (type *)alloc(sizeof(type))
+#define ALLOC_ONE_ID(type, id)  (type *)alloc_id(sizeof(type), id)
 #define ALLOC_MULT(type, count)  (type *)alloc(sizeof(type) * (count))
 #define ALLOC_CLEAR_ONE(type)  (type *)alloc_clear(sizeof(type))
+#define ALLOC_CLEAR_ONE_ID(type, id)  (type *)alloc_clear_id(sizeof(type), id)
 #define ALLOC_CLEAR_MULT(type, count)  (type *)alloc_clear(sizeof(type) * (count))
 #define LALLOC_CLEAR_ONE(type)  (type *)lalloc_clear(sizeof(type), FALSE)
 #define LALLOC_CLEAR_MULT(type, count)  (type *)lalloc_clear(sizeof(type) * (count), FALSE)