Обсуждение: BUG #4494: Memory leak in pg_regress.c

The following bug has been logged online:

Bug reference:      4494
Logged by:
Email address:      dvice_null@yahoo.com
PostgreSQL version: Latest cvs
Operating system:   Error in source code
Description:        Memory leak in pg_regress.c
Details:

In file src/test/regress/pg_regress.c:1112

It seems to me that variables "s" and "tmp" leak memory here if last_dot is
false:


static char *
get_alternative_expectfile(const char *expectfile, int i)
{
        char       *last_dot;
        int                     ssize = strlen(expectfile) + 2 + 1;
        char       *tmp = (char *) malloc(ssize);
        char       *s = (char *) malloc(ssize);

        strcpy(tmp, expectfile);
        last_dot = strrchr(tmp, '.');
        if (!last_dot)
                return NULL;
        *last_dot = '\0';
        snprintf(s, ssize, "%s_%d.%s", tmp, i, last_dot + 1);
        free(tmp);
        return s;
}

dvice_null@yahoo.com napsal(a):
> The following bug has been logged online:
>
> Bug reference:      4494
> Logged by:
> Email address:      dvice_null@yahoo.com
> PostgreSQL version: Latest cvs
> Operating system:   Error in source code
> Description:        Memory leak in pg_regress.c
> Details:
>
> In file src/test/regress/pg_regress.c:1112
>
> It seems to me that variables "s" and "tmp" leak memory here if last_dot is
> false:
>
>
> static char *
> get_alternative_expectfile(const char *expectfile, int i)
> {
>         char       *last_dot;
>         int                     ssize = strlen(expectfile) + 2 + 1;
>         char       *tmp = (char *) malloc(ssize);
>         char       *s = (char *) malloc(ssize);
>
>         strcpy(tmp, expectfile);
>         last_dot = strrchr(tmp, '.');
>         if (!last_dot)
>                 return NULL;
>         *last_dot = '\0';
>         snprintf(s, ssize, "%s_%d.%s", tmp, i, last_dot + 1);
>         free(tmp);
>         return s;
> }
>

OK. It seems as a bug. tmp and s should be freed when strrchr fails. Also there
is not check when malloc fails.

        Zdenek






--
Zdenek Kotala              Sun Microsystems
Prague, Czech Republic     http://sun.com/postgresql

Zdenek Kotala wrote:
> dvice_null@yahoo.com napsal(a):

>> In file src/test/regress/pg_regress.c:1112
>>
>> It seems to me that variables "s" and "tmp" leak memory here if last_dot is
>> false:

> OK. It seems as a bug. tmp and s should be freed when strrchr fails. Also
> there is not check when malloc fails.

So who's gonna send the patch?

--
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera napsal(a):
> Zdenek Kotala wrote:
>> dvice_null@yahoo.com napsal(a):
>
>>> In file src/test/regress/pg_regress.c:1112
>>>
>>> It seems to me that variables "s" and "tmp" leak memory here if last_dot is
>>> false:
>
>> OK. It seems as a bug. tmp and s should be freed when strrchr fails. Also
>> there is not check when malloc fails.
>
> So who's gonna send the patch?

I will do that. Still on my TODO.

        Zdenek

Patch written, attached, and applied.

---------------------------------------------------------------------------

dvice_null@yahoo.com wrote:
>
> The following bug has been logged online:
>
> Bug reference:      4494
> Logged by:
> Email address:      dvice_null@yahoo.com
> PostgreSQL version: Latest cvs
> Operating system:   Error in source code
> Description:        Memory leak in pg_regress.c
> Details:
>
> In file src/test/regress/pg_regress.c:1112
>
> It seems to me that variables "s" and "tmp" leak memory here if last_dot is
> false:
>
>
> static char *
> get_alternative_expectfile(const char *expectfile, int i)
> {
>         char       *last_dot;
>         int                     ssize = strlen(expectfile) + 2 + 1;
>         char       *tmp = (char *) malloc(ssize);
>         char       *s = (char *) malloc(ssize);
>
>         strcpy(tmp, expectfile);
>         last_dot = strrchr(tmp, '.');
>         if (!last_dot)
>                 return NULL;
>         *last_dot = '\0';
>         snprintf(s, ssize, "%s_%d.%s", tmp, i, last_dot + 1);
>         free(tmp);
>         return s;
> }
>
> --
> Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-bugs

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/test/regress/pg_regress.c
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/pg_regress.c,v
retrieving revision 1.56
diff -c -c -r1.56 pg_regress.c
*** src/test/regress/pg_regress.c    1 Jan 2009 17:24:04 -0000    1.56
--- src/test/regress/pg_regress.c    8 Jan 2009 20:03:29 -0000
***************
*** 1134,1140 ****
--- 1134,1144 ----
      strcpy(tmp, expectfile);
      last_dot = strrchr(tmp, '.');
      if (!last_dot)
+     {
+         free(tmp);
+         free(s);
          return NULL;
+     }
      *last_dot = '\0';
      snprintf(s, ssize, "%s_%d.%s", tmp, i, last_dot + 1);
      free(tmp);