<<< Date Index >>>     <<< Thread Index >>>

Re: [PATCH 2 of 4] Fix various resource leaks throughout sendlib.c



>> # HG changeset patch
>> # User Erik Hovland <erik@xxxxxxxxxxx>
>> # Date 1237420329 25200
>> # Branch HEAD
>> # Node ID 1b97d4b561846e0d77e8216bde94f7c2637a0149
>> # Parent  9ccf56c08a039ffa448fbd3f34c659a19fcb6ae4
>> Fix various resource leaks throughout sendlib.c
>
>> diff --git a/sendlib.c b/sendlib.c
>> --- a/sendlib.c
>> +++ b/sendlib.c
>> @@ -828,7 +828,7 @@
>>  for (i = 0; i < ncodes; i++)
>>    FREE (&tcode[i]);
>>
>> -  FREE (tcode);                /* __FREE_CHECKED__ */
>> +  FREE (&tcode);               /* __FREE_CHECKED__ */
>
> The same for the use of FREE(). This is probably wrong.

I'll have to look again. But if I recall tcode is allocated and since
FREE takes the pointer to the pointer, the & is required. But I am not
in front of my workstation right now. So I will defer to your
guidance.

>> int mutt_write_fcc (const char *path, HEADER *hdr, const char *msgid, int
>> post, char *fcc)
>> {
>> -  CONTEXT f;
>> +  CONTEXT *f = NULL;
>
> Here I don't understand what the leak should be. It just changes from
> stack to allocated memory causing more overhead I think. Can you explain
> why you change this?

Heh, the problem here is that the function that clears CONTEXT f
returns a pointer as an error indicator. But of course if a already
allocated CONTEXT is passed to it, then that error path is never
exercised. I contemplated changing that function so that it expected a
pre allocated CONTEXT but I took the easy way out and just didn't
allocate a CONTEXT and passed it an empty pointer. This allows the
return value to be checked and indicate whether we should proceed. But
I confess it was more of a concept rather then a fix.

E

-- 
Erik Hovland
erik@xxxxxxxxxxx
http://hovland.org/