From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3400 invoked by alias); 24 Oct 2007 17:57:53 -0000 Received: (qmail 3389 invoked by uid 22791); 24 Oct 2007 17:57:52 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 24 Oct 2007 17:57:48 +0000 Received: (qmail 28519 invoked from network); 24 Oct 2007 17:57:45 -0000 Received: from unknown (HELO localhost) (carlos@127.0.0.2) by mail.codesourcery.com with ESMTPA; 24 Oct 2007 17:57:45 -0000 Date: Wed, 24 Oct 2007 18:02:00 -0000 From: Carlos O'Donell To: Jim Blandy Cc: gdb-patches@sourceware.org, Daniel Jacobowitz Subject: Re: [PATCH] Fix uninitialized use of variables. Message-ID: <20071024175743.GP21241@lios> References: <20071020172137.GC28823@lios> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.16 (2007-06-11) Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-10/txt/msg00593.txt.bz2 On Tue, Oct 23, 2007 at 04:20:12PM -0700, Jim Blandy wrote: > > Carlos O'Donell writes: > > Index: gdb/remote.c > > =================================================================== > > RCS file: /cvs/src/src/gdb/remote.c,v > > retrieving revision 1.271 > > diff -u -p -r1.271 remote.c > > --- gdb/remote.c 8 Oct 2007 12:55:09 -0000 1.271 > > +++ gdb/remote.c 18 Oct 2007 16:34:05 -0000 > > @@ -1343,7 +1343,8 @@ unpack_varlen_hex (char *buff, /* packet > > static char * > > unpack_nibble (char *buf, int *val) > > { > > - ishex (*buf++, val); > > + if (!ishex (*buf++, val)) > > + error (_("Unpacked nibble does not contain hex characters.")); > > return buf; > > } > > This looks fine to me, although Daniel has thoughts on error handling > in the remote protocol that I don't fully understand. > > But the error message is going to be obscure to users. It should at > least say something about the remote protocol packet being misformed. In all truth I would have rather returned a status and let the caller determine the error message. Should the return of the function indicate status or number of nibbles parsed as ishex does? e.g. static int unpack_nibble (char **buf, int *val) { int cnt; (*buf)++; cnt = ishex (**buf, val); return cnt; } ... cnt = unpack_nibble (&pkt, &done); if (cnt != 1) error (_("Packet error: Unable to parse `done' from thread list response.!")); Cheers, Carlos. -- Carlos O'Donell CodeSourcery carlos@codesourcery.com (650) 331-3385 x716