From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pete Zaitcev Subject: Re: [PATCH v2 2/2] cld: read the cld.port file using g_file_get_contents Date: Sat, 28 Nov 2009 10:58:15 -0700 Message-ID: <20091128105815.7b5cd149@redhat.com> References: <1259375029-22050-1-git-send-email-cmccabe@alumni.cmu.edu> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1259375029-22050-1-git-send-email-cmccabe@alumni.cmu.edu> Sender: hail-devel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii" To: Colin McCabe Cc: Project Hail List , Jeff Garzik , Colin McCabe , zaitcev@redhat.com On Fri, 27 Nov 2009 18:23:49 -0800 Colin McCabe wrote: > Signed-off-by: Colin McCabe > + 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. -- Pete