Hi
3 remarks about vim/src/undo.c (at changeset: 271a5907f944):
(1) Using persistent-undo, I notice once in a while the following memory
leak, but I have not found the way to reproduce it all the time:
==3371== 10 bytes in 2 blocks are definitely lost in loss record 23 of 116
==3371== at 0x4024F70: malloc (vg_replace_malloc.c:236)
==3371== by 0x8114F98: lalloc (misc2.c:919)
==3371== by 0x81BCC06: u_read_undo (undo.c:983)
==3371== by 0x80C5359: readfile (fileio.c:2591)
==3371== by 0x8053976: open_buffer (buffer.c:132)
==3371== by 0x8098ABE: do_ecmd (ex_cmds.c:3658)
==3371== by 0x80AEF7B: do_exedit (ex_docmd.c:7620)
==3371== by 0x80AEC38: ex_edit (ex_docmd.c:7516)
==3371== by 0x80A782C: do_one_cmd (ex_docmd.c:2639)
==3371== by 0x80A5105: do_cmdline (ex_docmd.c:1108)
==3371== by 0x812AD39: nv_colon (normal.c:5226)
==3371== by 0x81245C3: normal_cmd (normal.c:1188)
==3371== by 0x80E7938: main_loop (main.c:1216)
==3371== by 0x80E742F: main (main.c:960)
It's this alloc in undo.c:
983 array = (char_u **)U_ALLOC_LINE(
984 (unsigned)(sizeof(char_u *) * uep->ue_size));
Looking at the undo.c code, memory seems to be properly freed
but I might be missing something since Valgrind does not generally
give spurious leak messages.
(2) In vim/src/undo.c I see at line 92:
91 /* See below: use malloc()/free() for memory management. */
92 #define U_USE_MALLOC 1
Whether U_USE_MALLOC is defined or not selects different
implementations of U_FREE_LINE and U_ALLOC_LINE.
Is the implementation when U_USE_MALLOC is undefined
still meant to be used? Or can it be removed?
I'm asking because if I comment out the line #define U_USE_MALLOC 1
at line undo.c:92, then Vim quickly crashes when using persistent-undo.
In other words, persistent undo only works when U_USE_MALLOC is defined.
(3) When U_USE_MALLOC is defined (default behavior), I see that 1 byte
is added to every allocations at line 117:
115 #ifdef U_USE_MALLOC
116 # define U_FREE_LINE(ptr) vim_free(ptr)
117 # define U_ALLOC_LINE(size) lalloc((long_u)((size) + 1), FALSE)
118 #else
This extra 1 byte is odd since it's not needed everywhere U_ALLOC_LINE(...)
is used. I think it's better if the caller is responsible for adding +1
when needed to avoid wasting memory. Attached patch does that.
Maybe this patch also fixes the leak (1) but I'm not sure since I don't
know how to reproduce it all the time. At least, I have not seen the
leak so far after applying the attached patch.
Regards
-- Dominique
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
diff -r 271a5907f944 src/undo.c
--- a/src/undo.c Tue May 25 22:09:21 2010 +0200
+++ b/src/undo.c Tue May 25 23:12:27 2010 +0200
@@ -114,7 +114,7 @@
#ifdef U_USE_MALLOC
# define U_FREE_LINE(ptr) vim_free(ptr)
-# define U_ALLOC_LINE(size) lalloc((long_u)((size) + 1), FALSE)
+# define U_ALLOC_LINE(size) lalloc((long_u)((size)), FALSE)
#else
static void u_free_line __ARGS((char_u *ptr, int keep));
static char_u *u_alloc_line __ARGS((unsigned size));
@@ -896,7 +896,7 @@
goto error;
else if (str_len > 0)
{
- if ((line_ptr = U_ALLOC_LINE(str_len)) == NULL)
+ if ((line_ptr = U_ALLOC_LINE(str_len + 1)) == NULL)
goto error;
for (i = 0; i < str_len; i++)
line_ptr[i] = (char_u)getc(fp);
@@ -917,10 +917,13 @@
/* uhp_table will store the freshly created undo headers we allocate
* until we insert them into curbuf. The table remains sorted by the
* sequence numbers of the headers. */
- uhp_table = (u_header_T **)U_ALLOC_LINE(num_head * sizeof(u_header_T *));
- if (uhp_table == NULL)
- goto error;
- vim_memset(uhp_table, 0, num_head * sizeof(u_header_T *));
+ if (num_head > 0)
+ {
+ uhp_table = (u_header_T **)U_ALLOC_LINE(num_head * sizeof(u_header_T *));
+ if (uhp_table == NULL)
+ goto error;
+ vim_memset(uhp_table, 0, num_head * sizeof(u_header_T *));
+ }
c = get2c(fp);
while (c == UF_HEADER_MAGIC)
@@ -980,14 +983,16 @@
uep->ue_lcount = get4c(fp);
uep->ue_size = get4c(fp);
uep->ue_next = NULL;
- array = (char_u **)U_ALLOC_LINE(
+
+ array = (uep->ue_size == 0)
+ ? NULL : (char_u **)U_ALLOC_LINE(
(unsigned)(sizeof(char_u *) * uep->ue_size));
for (i = 0; i < uep->ue_size; i++)
{
line_len = get4c(fp);
/* U_ALLOC_LINE provides an extra byte for the NUL terminator.*/
line = (char_u *)U_ALLOC_LINE(
- (unsigned) (sizeof(char_u) * line_len));
+ (unsigned) (sizeof(char_u) * line_len + 1));
if (line == NULL)
goto error;
for (j = 0; j < line_len; j++)
@@ -1100,8 +1105,7 @@
goto theend;
error:
- if (line_ptr != NULL)
- U_FREE_LINE(line_ptr);
+ U_FREE_LINE(line_ptr);
if (uhp_table != NULL)
{
for (i = 0; i < num_head; i++)
@@ -3074,7 +3078,7 @@
src = ml_get(lnum);
len = (unsigned)STRLEN(src);
- if ((dst = U_ALLOC_LINE(len)) != NULL)
+ if ((dst = U_ALLOC_LINE(len + 1)) != NULL)
mch_memmove(dst, src, (size_t)(len + 1));
return (dst);
}
