On 2011/4/14 16:28, Dominique Pellé wrote:
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).
I see I was wrong. Thanks.
Regards,
Hong Xu
2011/4/14
--
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
