Re: [HACKERS] Small memory leak in execute.c of ECPG driver
On 02/03/2015 02:45 PM, Michael Paquier wrote: On Tue, Feb 3, 2015 at 5:35 PM, Heikki Linnakangas hlinnakan...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Small memory leak in execute.c of ECPG driver
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Small memory leak in execute.c of ECPG driver
On Tue, Feb 3, 2015 at 5:35 PM, Heikki Linnakangas hlinnakan...@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 From 4e195f162d879bf563fec052710dd10ccdc4a89a Mon Sep 17 00:00:00 2001 From: Michael Paquier michael@otacoo.com Date: Tue, 3 Feb 2015 15:48:16 +0900 Subject: [PATCH] Fix memory leak in ecpg driver Issue pointed out by Coverity. --- src/interfaces/ecpg/ecpglib/execute.c | 16 1 file changed, 16 insertions(+) diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c index 8a3dd75..5d26af7 100644 --- a/src/interfaces/ecpg/ecpglib/execute.c +++ b/src/interfaces/ecpg/ecpglib/execute.c @@ -803,7 +803,10 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari mallocedval = quote_postgres(newcopy, quote, lineno); if (!mallocedval) + { + ecpg_free(newcopy); return false; + } *tobeinserted_p = mallocedval; } @@ -835,7 +838,10 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari mallocedval = quote_postgres(newcopy, quote, lineno); if (!mallocedval) + { + ecpg_free(newcopy); return false; + } *tobeinserted_p = mallocedval; } @@ -859,7 +865,10 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari nval = PGTYPESnumeric_new(); if (!nval) + { +ecpg_free(mallocedval); return false; + } if (var-type == ECPGt_numeric) result = PGTYPESnumeric_copy((numeric *) ((var + var-offset * element)-value), nval); @@ -869,6 +878,7 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari if (result != 0) { PGTYPESnumeric_free(nval); +ecpg_free(mallocedval); return false; } @@ -940,7 +950,10 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari { str = quote_postgres(PGTYPESinterval_to_asc((interval *) ((var + var-offset * element)-value)), quote, lineno); if (!str) + { +ecpg_free(mallocedval); return false; + } slen = strlen(str); if (!(mallocedval = ecpg_realloc(mallocedval, strlen(mallocedval) + slen + 2, lineno))) @@ -991,7 +1004,10 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari { str = quote_postgres(PGTYPESdate_to_asc(*(date *) ((var + var-offset * element)-value)), quote, lineno); if (!str) + { +ecpg_free(mallocedval); return false; + } slen = strlen(str); if (!(mallocedval = ecpg_realloc(mallocedval, strlen(mallocedval) + slen + 2, lineno))) -- 2.2.2 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] 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 From 5911fadddbf78d6d98f1d679e7ff2e78f9728185 Mon Sep 17 00:00:00 2001 From: Michael Paquier michael@otacoo.com Date: Tue, 3 Feb 2015 15:48:16 +0900 Subject: [PATCH] Fix memory leak in ecpg driver Issue pointed out by Coverity. --- src/interfaces/ecpg/ecpglib/execute.c | 9 + 1 file changed, 9 insertions(+) diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c index 8a3dd75..abe60a5 100644 --- a/src/interfaces/ecpg/ecpglib/execute.c +++ b/src/interfaces/ecpg/ecpglib/execute.c @@ -859,7 +859,10 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari nval = PGTYPESnumeric_new(); if (!nval) + { +ecpg_free(mallocedval); return false; + } if (var-type == ECPGt_numeric) result = PGTYPESnumeric_copy((numeric *) ((var + var-offset * element)-value), nval); @@ -940,7 +943,10 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari { str = quote_postgres(PGTYPESinterval_to_asc((interval *) ((var + var-offset * element)-value)), quote, lineno); if (!str) + { +ecpg_free(mallocedval); return false; + } slen = strlen(str); if (!(mallocedval = ecpg_realloc(mallocedval, strlen(mallocedval) + slen + 2, lineno))) @@ -991,7 +997,10 @@ ecpg_store_input(const int lineno, const bool force_indicator, const struct vari { str = quote_postgres(PGTYPESdate_to_asc(*(date *) ((var + var-offset * element)-value)), quote, lineno); if (!str) + { +ecpg_free(mallocedval); return false; + } slen = strlen(str); if (!(mallocedval = ecpg_realloc(mallocedval, strlen(mallocedval) + slen + 2, lineno))) -- 2.2.2 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers