SetVariable
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
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:
Show quoted text
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
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"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
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
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
"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
"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
"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
"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_XSTRDUPNot 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
"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
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"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 applyedSure.
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