From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jeff Garzik <jeff@garzik.org>
Subject: Re: [PATCH v2 2/2] cld: read the cld.port file using g_file_get_contents
Date: Sat, 28 Nov 2009 05:37:16 -0500
Message-ID: <4B10FD5C.8040705@garzik.org>
References: <1259375029-22050-1-git-send-email-cmccabe@alumni.cmu.edu>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Return-path: <hail-devel-owner@vger.kernel.org>
In-Reply-To: <1259375029-22050-1-git-send-email-cmccabe@alumni.cmu.edu>
Sender: hail-devel-owner@vger.kernel.org
List-ID: <hail-devel.vger.kernel.org>
Content-Type: text/plain; charset="us-ascii"; format="flowed"
To: Colin McCabe <cmccabe@alumni.cmu.edu>
Cc: Project Hail List <hail-devel@vger.kernel.org>, Pete Zaitcev <zaitcev@redhat.com>

On 11/27/2009 09:23 PM, Colin McCabe wrote:
> Signed-off-by: Colin McCabe<cmccabe@alumni.cmu.edu>
>
> ---
>   lib/common.c |   29 +++++++++++++++--------------
>   1 files changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/lib/common.c b/lib/common.c
> index 68f60f8..db20e2a 100644
> --- a/lib/common.c
> +++ b/lib/common.c
> @@ -1,4 +1,5 @@
>
> +#include<stdio.h>
>   #include<stdlib.h>
>   #include<fcntl.h>
>   #include<unistd.h>
> @@ -61,26 +62,26 @@ const char *cld_errstr(enum cle_err_codes ecode)
>    */
>   int cld_readport(const char *fname)
>   {
> -	enum { LEN = 11 };
> -	char buf[LEN+1];
>   	long port;
> -	int fd;
> -	int rc;
> +	gchar *buf;
> +	GError *err = NULL;
> +	gsize len;
>
> -	if ((fd = open(fname, O_RDONLY)) == -1)
> -		return -errno;
> -	rc = read(fd, buf, LEN);
> -	close(fd);
> -	if (rc<  0)
> -		return -errno;
> -	if (rc == 0)
> -		return -EPIPE;
> -	buf[rc] = 0;
> +	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;
> +	}
>
> +	if (len == 0) {
> +		g_free(buf);
> +		return -EPIPE;
> +	}
>   	port = strtol(buf, NULL, 10);
> +	g_free(buf);

Two added wrinkles, unfortunately:

1) 'buf' is no longer nul-terminated, which means strtol() has become a 
buffer overrun.

2) this is used in daemons, so we cannot make assumptions about error 
output.  The best you can do is return a useful numeric error code, 
because fprintf(stderr,...) does nothing in a daemon that redirected 
stderr to null.