Hi all, 
I found this code on the file variables.c and
in the function SetVariable I read:

if (strcmp(current->name, name) == 0)
{       
           free(current->value);
           current->value = strdup(value);
           return current->value ? true : false;
}

this mean that if there is no memory left on the
sistem we loose the old value, 
if this is not the indeended behaviour may be is better do:

if (strcmp(current->name, name) == 0)
{       
           char * tmp_value = strdup(value);

           if ( !tmp_value ) 
           {
                  return false;
           }
      
           free(current->value);
           current->value = tmp_value;

           return true;
}


Regards
Gaetano Mendola


Wed, 27 Aug 2003 16:58:50 +0200
"Mendola Gaetano" <mendola@bigfoot.com>
SetVariable 
Just a follow up, 
is it better to give a patch for this kind of stuff ?

Regards
Gaetano Mendola

""Mendola Gaetano"" <mendola@bigfoot.com> wrote:
> Hi all, 
> I found this code on the file variables.c and
> in the function SetVariable I read:
> 
> if (strcmp(current->name, name) == 0)
> {       
>            free(current->value);
>            current->value = strdup(value);
>            return current->value ? true : false;
> }
> 
> this mean that if there is no memory left on the
> sistem we loose the old value, 
> if this is not the indeended behaviour may be is better do:
> 
> if (strcmp(current->name, name) == 0)
> {       
>            char * tmp_value = strdup(value);
> 
>            if ( !tmp_value ) 
>            {
>                   return false;
>            }
>       
>            free(current->value);
>            current->value = tmp_value;
> 
>            return true;
> }
> 
> 
> Regards
> Gaetano Mendola
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 8: explain analyze is your friend
> 


Thu, 28 Aug 2003 19:58:09 +0200
"Mendola Gaetano" <mendola@bigfoot.com>
Re: SetVariable
Mendola Gaetano wrote:
> Hi all, 
> I found this code on the file variables.c and
> in the function SetVariable I read:
> 
> if (strcmp(current->name, name) == 0)
> {       
>            free(current->value);
>            current->value = strdup(value);
>            return current->value ? true : false;
> }
> 
> this mean that if there is no memory left on the
> sistem we loose the old value, 
> if this is not the indeended behaviour may be is better do:

I see other strdup() calls that don't check on a return.  Should we deal
with those too?


> 
> if (strcmp(current->name, name) == 0)
> {       
>            char * tmp_value = strdup(value);
> 
>            if ( !tmp_value ) 
>            {
>                   return false;
>            }
>       
>            free(current->value);
>            current->value = tmp_value;
> 
>            return true;
> }
> 
> 
> Regards
> Gaetano Mendola
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 8: explain analyze is your friend
> 

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Sat, 30 Aug 2003 00:12:24 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: SetVariable
"Bruce Momjian" <pgman@candle.pha.pa.us> wrote:
> I see other strdup() calls that don't check on a return.  Should we deal
> with those too?

Well strdup obtain the memory for the new string using a malloc
and normally is a good habit check the return value of a malloc.

Regards
Gaetano Mendola


Sat, 30 Aug 2003 08:29:01 +0200
"Gaetano Mendola" <mendola@bigfoot.com>
Re: SetVariable
Gaetano Mendola wrote:
> "Bruce Momjian" <pgman@candle.pha.pa.us> wrote:
> > I see other strdup() calls that don't check on a return.  Should we deal
> > with those too?
> 
> Well strdup obtain the memory for the new string using a malloc
> and normally is a good habit check the return value of a malloc.

Right.  My point is that we have lots of other strdup's in the code. 
Should we fix those too?  Seems we should be consistent.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Sat, 30 Aug 2003 09:58:56 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: SetVariable
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I see other strdup() calls that don't check on a return.  Should we deal
> with those too?

strdup -> xstrdup if you're concerned.

			regards, tom lane

Sat, 30 Aug 2003 11:03:40 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: SetVariable 
"Bruce Momjian" <pgman@candle.pha.pa.us> wrote:
> Gaetano Mendola wrote:
> > "Bruce Momjian" <pgman@candle.pha.pa.us> wrote:
> > > I see other strdup() calls that don't check on a return.  Should we
deal
> > > with those too?
> >
> > Well strdup obtain the memory for the new string using a malloc
> > and normally is a good habit check the return value of a malloc.
>
> Right.  My point is that we have lots of other strdup's in the code.
> Should we fix those too?  Seems we should be consistent.

Of course yes, consider also that inside SetVariable the check is
performed but too late, after that the old value was loose for ever.

Keep also the suggestion of Tom Late  about the xstrdup.

Regards
Gaetano Mendola



Sat, 30 Aug 2003 19:58:03 +0200
"Gaetano Mendola" <mendola@bigfoot.com>
Re: SetVariable
"Tom Lane" <tgl@sss.pgh.pa.us> wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > I see other strdup() calls that don't check on a return.  Should we deal
> > with those too?
>
> strdup -> xstrdup if you're concerned.

May be is a good idea avoid the future use:

#define strdup   STRDUP_DEPRECATED_USE_INSTEAD_XSTRDUP

I don't like the other solution: redefine the strdup in function of xstrdup.


Regards
Gaetano Mendola



Sat, 30 Aug 2003 20:07:49 +0200
"Gaetano Mendola" <mendola@bigfoot.com>
Re: SetVariable
"Gaetano Mendola" <mendola@bigfoot.com> writes:
> "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
>> strdup -> xstrdup if you're concerned.

> May be is a good idea avoid the future use:
> #define strdup   STRDUP_DEPRECATED_USE_INSTEAD_XSTRDUP

Not a good idea --- there are places that want to check for strdup
failure and do something different than exit(1).

			regards, tom lane

Sat, 30 Aug 2003 17:30:30 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: SetVariable 
"Tom Lane" <tgl@sss.pgh.pa.us> wrote:
> "Gaetano Mendola" <mendola@bigfoot.com> writes:
> > "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
> >> strdup -> xstrdup if you're concerned.
> 
> > May be is a good idea avoid the future use:
> > #define strdup   STRDUP_DEPRECATED_USE_INSTEAD_XSTRDUP
> 
> Not a good idea --- there are places that want to check for strdup
> failure and do something different than exit(1).

That's true.

Regards
Gaetano Mendola


Sun, 31 Aug 2003 01:27:49 +0200
"Gaetano Mendola" <mendola@bigfoot.com>
Re: SetVariable
"Bruce Momjian" <pgman@candle.pha.pa.us> wrote:
> Gaetano Mendola wrote:
> > "Bruce Momjian" <pgman@candle.pha.pa.us> wrote:
> > > I see other strdup() calls that don't check on a return.  Should we
deal
> > > with those too?
> >
> > Well strdup obtain the memory for the new string using a malloc
> > and normally is a good habit check the return value of a malloc.
>
> Right.  My point is that we have lots of other strdup's in the code.
> Should we fix those too?  Seems we should be consistent.

Shall I post the patch for SetVariable ?

I know that this change is not so important but I want to try
on my skin the cycle
seen somethink wrong => send patch => be applyed

Regards
Gaetano Mendola



Mon, 1 Sep 2003 18:55:19 +0200
"Mendola Gaetano" <mendola@bigfoot.com>
Re: SetVariable
Mendola Gaetano wrote:
> "Bruce Momjian" <pgman@candle.pha.pa.us> wrote:
> > Gaetano Mendola wrote:
> > > "Bruce Momjian" <pgman@candle.pha.pa.us> wrote:
> > > > I see other strdup() calls that don't check on a return.  Should we
> deal
> > > > with those too?
> > >
> > > Well strdup obtain the memory for the new string using a malloc
> > > and normally is a good habit check the return value of a malloc.
> >
> > Right.  My point is that we have lots of other strdup's in the code.
> > Should we fix those too?  Seems we should be consistent.
> 
> Shall I post the patch for SetVariable ?
> 
> I know that this change is not so important but I want to try
> on my skin the cycle
> seen somethink wrong => send patch => be applyed

Sure.

It would be good if you would evaluate all the strdups, find the ones
that don't check properly, and submit a big patch of all those.  The fix
can be to call xstrdup, or to check the return code.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Mon, 1 Sep 2003 13:02:06 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: SetVariable
"Bruce Momjian" <pgman@candle.pha.pa.us> wrote:
> Mendola Gaetano wrote:
> > "Bruce Momjian" <pgman@candle.pha.pa.us> wrote:
> > > Gaetano Mendola wrote:
> > > > "Bruce Momjian" <pgman@candle.pha.pa.us> wrote:
> > > > > I see other strdup() calls that don't check on a return.  Should
we
> > deal
> > > > > with those too?
> > > >
> > > > Well strdup obtain the memory for the new string using a malloc
> > > > and normally is a good habit check the return value of a malloc.
> > >
> > > Right.  My point is that we have lots of other strdup's in the code.
> > > Should we fix those too?  Seems we should be consistent.
> >
> > Shall I post the patch for SetVariable ?
> >
> > I know that this change is not so important but I want to try
> > on my skin the cycle
> > seen somethink wrong => send patch => be applyed
>
> Sure.
>
> It would be good if you would evaluate all the strdups, find the ones
> that don't check properly, and submit a big patch of all those.  The fix
> can be to call xstrdup, or to check the return code.

I will.

Regards
Gaetano Mendola



Tue, 2 Sep 2003 00:34:48 +0200
"Gaetano Mendola" <mendola@bigfoot.com>
Re: SetVariable