[vim/vim] Realloc memory not freed after usage (memory leak), and extra (possible redundant code) (#795)

62 views
Skip to first unread message

David Silveiro

unread,
May 10, 2016, 10:50:35 PM5/10/16
to vim/vim

Hey @brammool just chucking a few more things your way, hope you don't mind ^^

On line 2153 in /src/gui_photon.c, utf8_buffer isn't freed from memory after usage.

Alongside that, I noticed that if true condition on line 2151 is met, isn't len & s a little redundant if said condition holds true, on line 2165 & 2164


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub

Bram Moolenaar

unread,
May 12, 2016, 1:02:50 AM5/12/16
to vim/vim

David Silveiro wrote:

> Hey @brammool just chucking a few more things your way, hope you don't mind ^^
>
> On line **2153 in /src/gui_photon.c**, **utf8_buffer** isn't freed from memory after usage.

This variable is static, so it remains used throughout the life of the
process. That's not a leak.

> Alongside that, I noticed that if true condition on line **2151** is met, isn't **len** & **s** a little redundant if said condition holds true, on line **2165** & **2164**

Not sure what you mean.

--
CART DRIVER: Bring out your dead!
LARGE MAN: Here's one!
CART DRIVER: Ninepence.
BODY: I'm not dead!
"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

/// 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 ///

David Silveiro

unread,
May 15, 2016, 3:21:30 PM5/15/16
to vim/vim

@brammool Ah, didn't notice!

My question in regards to the redundancy:


#endif
       )
    {
    int src_taken, dst_made; 

    /* Use a static buffer to avoid large amounts of de/allocations */
    if (utf8_len < len)
    {
        utf8_buffer = realloc(utf8_buffer, len * MB_LEN_MAX);
        utf8_len = len;
    }

    PxTranslateToUTF(
        charset_translate,
        s,
        len,
        &src_taken,
        utf8_buffer,
        utf8_len,
        &dst_made);
    s = utf8_buffer;
    len = dst_made; 
    }

    PgDrawText(s, len, &pos, 0);

Q1

We create the variable dst_made on line 2148, then assign len on line 2166, considering it's existence unaltered before, is this line not redundant?

Q2

If the condition on line 5151 is met, and we have the utf8_buffer on line 5153,
isn't the assignment of s unneeded? Because as is the same if the condition returns false. utf8_buffer maintains its NULL or realloc data in the end, renamings a bit strange.

Bram Moolenaar

unread,
May 20, 2016, 5:18:11 AM5/20/16
to vim/vim

> @brammool Ah, didn't notice!
>
> My question in regards to the redundancy:
>
>
> ```
>
> #endif
> )
> {
> int src_taken, dst_made;
>
> /* Use a static buffer to avoid large amounts of de/allocations */
> if (utf8_len < len)
> {
> utf8_buffer = realloc(utf8_buffer, len * MB_LEN_MAX);
> utf8_len = len;
> }
>
> PxTranslateToUTF(
> charset_translate,
> s,
> len,
> &src_taken,
> utf8_buffer,
> utf8_len,
> &dst_made);
> s = utf8_buffer;
> len = dst_made;
> }
>
> PgDrawText(s, len, &pos, 0);
>
> ```
>
> **Q1**

>
> We create the variable dst_made on line 2148, then assign len on line
> 2166, considering it's existence unaltered before, is this line not
> redundant?

Doesn't the PxTranslateToUTF function set dst_made to another value in
certain circumstances? I don't know this function, thus I can't tell
for sure.

> **Q2**

>
> If the condition on line 5151 is met, and we have the utf8_buffer on line 5153,
> isn't the assignment of **s** unneeded? Because as is the same if the

> condition returns false. utf8_buffer maintains its NULL or realloc
> data in the end, renamings a bit strange.

These line numbers are off. I assume that some translation was done,
thus s must point to the translated result.

--
FIRST VILLAGER: We have found a witch. May we burn her?

"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

/// 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,
Jun 28, 2016, 4:09:26 PM6/28/16
to vim/vim

Closed #795.


You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub, or mute the thread.

Christian Brabandt

unread,
Jun 28, 2016, 4:09:26 PM6/28/16
to vim/vim

i am closing this.


You are receiving this because you are subscribed to this thread.

Reply all
Reply to author
Forward
0 new messages