Re: [PATCH v2 2/2] cld: read the cld.port file using g_file_get_contents

[Jeff Garzik <jeff@garzik.org>]

On 11/28/2009 12:58 PM, Pete Zaitcev wrote:
> On Fri, 27 Nov 2009 18:23:49 -0800
> Colin McCabe<cmccabe@alumni.cmu.edu>  wrote:
>
>> Signed-off-by: Colin McCabe<cmccabe@alumni.cmu.edu>
>> +	if (!g_file_get_contents(fname,&buf,&len,&err)) {
>> +		fprintf(stderr, "Unable to read port file: %s\n",
>> +			 err->message);
>> +		g_error_free(err);
>> +		return -EIO;
>> +	}
>
> We should not print errors from library routines, because it
> leads to double-printing (so even if you swap the fprintf to
> applog, it's still not a good thing to do).
>
> Otherwise seems fine at first blush.
>
> BTW, speaking of double-something, here's a funny buglet:
>
> diff --git cld/server/server.c cld-tip/server/server.c
> --- cld/server/server.c
> +++ cld-tip/server/server.c
> @@ -722,7 +741,7 @@ static int net_open_any(void)
>
>   	if (cld_srv.port_file) {
>   		char portstr[7];
> -		snprintf(portstr, sizeof(portstr), "%u\n", port);
> +		snprintf(portstr, sizeof(portstr), "%u", port);
>   		return net_write_port(cld_srv.port_file, portstr);
>   	}
>   	return 0;
>
> Our PID files contain a linefeed, as a courtesy to those who
> examine them from a shell. But CLD's contains two.
>

Agreed... wanna wrap that in a signed-off-by?  :)

	Jeff