Thread: Small memory leak in execute.c of ECPG driver
Hi all, In exactly 3 places of the ECPG driver (for numeric, for interval and for date), we do something as follows: /* Allocation of mallocedval */ if (!(mallocedval = ecpg_strdup("array [", lineno))) return false; for (element = 0; element < var->arrsize; element++) { int result; ptr = stuff_alloc(); if (!ptr) return false; <= Leak here of mallocedval It happens that if the allocation done within this for loop fails we leak mallocedval that was previously allocated. Attached is a patch to fix this issue spotted by Coverity. Regards -- Michael
Attachment
On 02/03/2015 08:58 AM, Michael Paquier wrote: > Hi all, > > In exactly 3 places of the ECPG driver (for numeric, for interval and > for date), we do something as follows: > /* Allocation of mallocedval */ > if (!(mallocedval = ecpg_strdup("array [", lineno))) > return false; > > for (element = 0; element < var->arrsize; element++) > { > int result; > > ptr = stuff_alloc(); > if (!ptr) > return false; <= Leak here of mallocedval > > It happens that if the allocation done within this for loop fails we > leak mallocedval that was previously allocated. Attached is a patch to > fix this issue spotted by Coverity. I think there are more similar leaks nearby. After the first hunk, there's another if-check with "return false" that also leaks mallocedval. Right after the two other hunks, if the ecpg_realloc fails, we again leak mallocedval. I wonder why Coverity didn't warn about those? Maybe it would've, after fixing the first ones. - Heikki
On Tue, Feb 3, 2015 at 5:35 PM, Heikki Linnakangas <hlinnakangas@vmware.com> wrote: > I think there are more similar leaks nearby. After the first hunk, there's > another if-check with "return false" that also leaks mallocedval. Right > after the two other hunks, if the ecpg_realloc fails, we again leak > mallocedval. Yes, I found some extra ones by re-reading the code again with newcopy (2) as well as mallocedval (1) as you mentioned. > I wonder why Coverity didn't warn about those? Maybe it would've, after > fixing the first ones. Hard to say. Perhaps it gives up after finding one failure in a code path, or perhaps it would have found it after a version update.. In any case, an updated patch is attached. -- Michael
Attachment
On 02/03/2015 02:45 PM, Michael Paquier wrote: > On Tue, Feb 3, 2015 at 5:35 PM, Heikki Linnakangas > <hlinnakangas@vmware.com> wrote: >> >I think there are more similar leaks nearby. After the first hunk, there's >> >another if-check with "return false" that also leaks mallocedval. Right >> >after the two other hunks, if the ecpg_realloc fails, we again leak >> >mallocedval. > Yes, I found some extra ones by re-reading the code again with newcopy > (2) as well as mallocedval (1) as you mentioned. Committed, along with more fixes for leaks on ecpg_realloc failures. - Heikki