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

[olafBuddenhagen]

Hi,

On Fri, Aug 15, 2008 at 06:27:00PM +0200, zhengda wrote:
> olafBuddenhagen@gmx.net wrote:

>>> 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...
>>   
> Right, it cannot avoid bugs, but I believe it is more likely expose
> the  bugs in the program.

It really depends on the context. In some cases -- if the pointer is
dereferenced -- NULL will create an obvious failure, while some random
uninitialized pointer might be harder to track down.

In other cases -- if it's used as argument to realloc(), free() etc.; or
if it's tested as a result from some other operation (getenv() in this
case) -- NULL will be treated as perfectly valid, and not give any
indication of failure at all!

Thus I don't think it's good practice in general to initialize all
pointers to NULL...

> 2008-06-30 Zheng Da <zhengda1936@gmail.com>
>
>    * hurd/hurdsocks.c (_hurd_socket_server): Searches environment variables
>    for the socket server insteading of using the default one.
>    (SOCK_SERV_%d, SOCK_SERV): The environment variables that contains
>    the path of the socket server.

As was pointed out by someone in the earlier discussion, we don't use
the term "path" in this context. SOCK_SERV_%d contains a file name,
SOCK_SERV a directory name.

Perhaps you could just say "location" instead of "path"...

(BTW, I still think that SOCK_SERV_DIR for the latter would be
clearer... But I don't insist on this if you prefer it as is :-) )

> -  socket_t server;
> +  socket_t server = MACH_PORT_NULL;

This is not necessary anymore, as "server" will be always assigned,
unless we error out earlier...

> +      char *np = NULL;

Err... I thought it was obvious that what I said about initializing
variables would also apply here...

> +      char *sock_servs = NULL;

...and here as well.

> +      if (__asprintf (&name, "SOCK_SERV_%d", domain) < 0)
> +    __libc_fatal ("hurd: Can't get the socket server path\n");

What happened to indentation here?!

-antrik-