Re: Collection of memory leaks for ECPG driver

Поиск
Список
Период
Сортировка
От Michael Meskes
Тема Re: Collection of memory leaks for ECPG driver
Дата
Msg-id 20150608122227.GA26885@feivel.credativ.lan
обсуждение исходный текст
Ответ на Collection of memory leaks for ECPG driver  (Michael Paquier <michael.paquier@gmail.com>)
Ответы Re: Collection of memory leaks for ECPG driver
Список pgsql-hackers
On Mon, Jun 08, 2015 at 01:57:32PM +0900, Michael Paquier wrote:
> Please find attached a patch aimed at fixing a couple of memory leaks
> in the ECPG driver. Coverity (and sometimes I after reading some other
> code paths) found them.

And some are not correct it seems. But then some of the code isn't either. :)

> diff --git a/src/interfaces/ecpg/compatlib/informix.c b/src/interfaces/ecpg/compatlib/informix.c
> index d6de3ea..c1e3dfb 100644
> --- a/src/interfaces/ecpg/compatlib/informix.c
> +++ b/src/interfaces/ecpg/compatlib/informix.c
> @@ -672,6 +672,7 @@ intoasc(interval * i, char *str)
>      if (!str)
>          return -errno;
>  
> +    free(str);
>      return 0;
>  }

This function never worked it seems, we need to use a temp string, copy it to str and then free the temp one.

> diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c
> index 55c5680..12f445d 100644
> --- a/src/interfaces/ecpg/ecpglib/connect.c
> +++ b/src/interfaces/ecpg/ecpglib/connect.c
> @@ -298,7 +298,8 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
>          envname = getenv("PG_DBPATH");
>          if (envname)
>          {
> -            ecpg_free(dbname);
> +            if (dbname)
> +                ecpg_free(dbname);
>              dbname = ecpg_strdup(envname, lineno);
>          }

This is superfluous, at least with glibc. free()'s manpage clearly says "If
ptr is NULL, no operation is performed.", so why should we check again? Or do
we have architectures where free(NULL) is not a noop?

> diff --git a/src/interfaces/ecpg/preproc/descriptor.c b/src/interfaces/ecpg/preproc/descriptor.c
> index 053a7af..ebd95d3 100644
> --- a/src/interfaces/ecpg/preproc/descriptor.c
> +++ b/src/interfaces/ecpg/preproc/descriptor.c

Do we really have to worry about these in a short running application like a precompiler, particularly if they add more
thana simple free() command?
 

But then, you already did the work, so why not. Anyway, please tell me if you want to update the patch or if I should
commitwhatever is clear already.
 

Michael

-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at gmail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Dean Rasheed
Дата:
Сообщение: Re: CREATE POLICY and RETURNING
Следующее
От: David Gould
Дата:
Сообщение: Re: [CORE] Restore-reliability mode