H Xu wrote:
> On 2011/4/14 3:05, Dominique Pellé wrote:
>>
>> Hi
>>
>> Attached patch fixes a bug in vim_getenv() found
>> with cppcheck static analyzer:
>>
>> $ cd vim/src ; cppcheck -f -I proto misc1.c
>> [misc1.c:3956]: (error) Possible null pointer dereference: mustfree -
>> otherwise it is redundant to check if mustfree is null at line 3953
>>
>> Reading the code, I'd expect invalid memory free to possibly happen
>> on Windows. I don't have a Windows computer to test it, but
>> the patch is simple anyway.
>
> So the line below, which is "*mustfree = TRUE;", should be useless?
No. I don't think so. The line "*mustfree = TRUE;" is needed.
Code before patch (as in Vim-7.3.161):
3876 char_u *
3877 vim_getenv(name, mustfree)
3878 char_u *name;
3879 int *mustfree; /* set to TRUE when returned is
allocated */
3880 {
...
3950 acp_to_enc(p, (int)STRLEN(p), &pp, &len);
3951 if (pp != NULL)
3952 {
3953 if (mustfree)
3954 vim_free(p);
3955 p = pp;
3956 *mustfree = TRUE;
3957 }
This was clearly wrong since mustfree is a pointer. So
test at line 3953 was always true since we never
call vim_getenv with mustfree being NULL.
The patch changes it into (added * at line 3953):
3876 char_u *
3877 vim_getenv(name, mustfree)
3878 char_u *name;
3879 int *mustfree; /* set to TRUE when returned is
allocated */
3880 {
...
3950 acp_to_enc(p, (int)STRLEN(p), &pp, &len);
3951 if (pp != NULL)
3952 {
3953 if (*mustfree)
3954 vim_free(p);
3955 p = pp;
3956 *mustfree = TRUE;
3957 }
And line 3956 is needed I think since 'p' has been freed and
replaced by pp. Caller will be responsible for freeing it (hence
*mustfree must be set to true at line 3956).
Now looking at function acp_to_enc(...), I also see another bug:
line 3950 may not always initialize pp. Yet we use pp at line
3951. So I think 'pp' should also be initialized to NULL before calling
acp_to_encp(...), or acp_to_encp(...) should set 'out' parameter to NULL
when it fails. The same bug happens in several other places where
acp_to_enc(...) is called. I can propose a patch, but I don't have
a Windows environment to test it (bug is specific to Windows).
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
