Re: The patch of glibc which allows the user to override the pfinet server

[olafBuddenhagen]

Hi,

On Wed, Aug 13, 2008 at 12:41:12PM +0200, zhengda wrote:
> olafBuddenhagen@gmx.net wrote:
>> On Tue, Aug 12, 2008 at 08:05:49PM +0200, zhengda wrote:

>>> +      char *name = NULL;
>>>     
>>
>> No need to initialize this -- the asprintf() will overwrite it anyways.
>>   
> But isn't it always right to initialize a local variable to reduce the  
> possibility of getting bugs?

It won't avoid bugs; only either cover them up, or (less likely) expose
them more clearly -- depending on context...

I for my part consider such redundant code rather confusing -- don't
know about others.

>>> +      if (__asprintf (&name, "SOCK_SERV_%d", domain) > 0)
>>> +        {
>>> +          np = getenv (name);
>>> +          __free (name);
>>> +        }
>>
>> You need to check whether asprintf() returns an error code (-1), meaning
>> memory allocation failed, and handle it accordingly -- just like you
>> handled failure with the explicit malloc() in your original code.
>>   
> as I understand from the manual of asprintf, asprintf always returns -1  
> if there is some error.

Yes. And if it does, you need to abort, rather than ignoring it...

>>> +          if (__asprintf (&name, "%s/%d", sock_servs, domain) > 0)
>>> +              np = name;
>>>     
>>
>> I think you can print to np directly here...

> I need to free the space afterwards.
> If I use np directly, I'll not be able to figure out whether I should  
> free the memory because np might point to the environment variable.

Indeed, you are right :-)

-antrik-