Patch 7.3.746

44 views
Skip to first unread message

Bram Moolenaar

unread,
Dec 5, 2012, 9:17:08 AM12/5/12
to vim...@googlegroups.com

Patch 7.3.746
Problem: Memory leaks when using location lists.
Solution: Set qf_title to something. (Christian Brabandt)
Files: src/eval.c, src/quickfix.c


*** ../vim-7.3.745/src/eval.c 2012-10-21 23:55:59.000000000 +0200
--- src/eval.c 2012-12-05 14:47:56.000000000 +0100
***************
*** 16292,16298 ****
action = *act;
}

! if (l != NULL && set_errorlist(wp, l, action, NULL) == OK)
rettv->vval.v_number = 0;
}
#endif
--- 16292,16299 ----
action = *act;
}

! if (l != NULL && set_errorlist(wp, l, action,
! (char_u *)(wp == NULL ? "setqflist()" : "setloclist()")) == OK)
rettv->vval.v_number = 0;
}
#endif
*** ../vim-7.3.745/src/quickfix.c 2012-11-28 22:12:40.000000000 +0100
--- src/quickfix.c 2012-12-05 14:51:52.000000000 +0100
***************
*** 2124,2138 ****
int idx;
{
qfline_T *qfp;

while (qi->qf_lists[idx].qf_count)
{
qfp = qi->qf_lists[idx].qf_start->qf_next;
! if (qi->qf_lists[idx].qf_title != NULL)
{
vim_free(qi->qf_lists[idx].qf_start->qf_text);
vim_free(qi->qf_lists[idx].qf_start->qf_pattern);
vim_free(qi->qf_lists[idx].qf_start);
}
qi->qf_lists[idx].qf_start = qfp;
--qi->qf_lists[idx].qf_count;
--- 2124,2145 ----
int idx;
{
qfline_T *qfp;
+ int stop = FALSE;

while (qi->qf_lists[idx].qf_count)
{
qfp = qi->qf_lists[idx].qf_start->qf_next;
! if (qi->qf_lists[idx].qf_title != NULL && !stop)
{
vim_free(qi->qf_lists[idx].qf_start->qf_text);
+ stop = (qi->qf_lists[idx].qf_start == qfp);
vim_free(qi->qf_lists[idx].qf_start->qf_pattern);
vim_free(qi->qf_lists[idx].qf_start);
+ if (stop)
+ /* Somehow qf_count may have an incorrect value, set it to 1
+ * to avoid crashing when it's wrong.
+ * TODO: Avoid qf_count being incorrect. */
+ qi->qf_lists[idx].qf_count = 1;
}
qi->qf_lists[idx].qf_start = qfp;
--qi->qf_lists[idx].qf_count;
*** ../vim-7.3.745/src/version.c 2012-12-05 14:42:56.000000000 +0100
--- src/version.c 2012-12-05 15:15:45.000000000 +0100
***************
*** 727,728 ****
--- 727,730 ----
{ /* Add new patch number below this line */
+ /**/
+ 746,
/**/

--
hundred-and-one symptoms of being an internet addict:
98. The Alta Vista administrators ask you what sites are missing
in their index files.

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

Christian Brabandt

unread,
Dec 7, 2012, 2:47:38 AM12/7/12
to vim...@googlegroups.com
On Wed, December 5, 2012 15:17, Bram Moolenaar wrote:
> Patch 7.3.746
[�]
> + /* Somehow qf_count may have an incorrect value, set it to 1
> + * to avoid crashing when it's wrong.
> + * TODO: Avoid qf_count being incorrect. */
> + qi->qf_lists[idx].qf_count = 1;

I finally found the root cause of having qf_count being wrong.

The vimgrep doesn't expect, that the quickfix list changes between
loading files and uses qf_add_entry() to add entries, which happily
increments qf_count of the wrong curlist, while the actual entries
will always be attached at the correct position to which prevp points
to.

I think, this patch prevents it, by checking that qi->curlist is
always the one, vimgrep expects. But please check. I am not entirely
sure this works correctly in all situations, but at least my tests
have all been successful so far (valgrind did not report errors, the
test suite runs as expected and the crashes are avoided)

regards,
Christian
vimgrep_autocmd_corruptions.diff

Bram Moolenaar

unread,
Dec 7, 2012, 3:37:18 PM12/7/12
to Christian Brabandt, vim...@googlegroups.com
Thanks. I'll add it near the top of the todo list.

--
hundred-and-one symptoms of being an internet addict:
120. You ask a friend, "What's that big shiny thing?" He says, "It's the sun."
Reply all
Reply to author
Forward
0 new messages