Обсуждение: 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

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

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