Re: [PATCH] Fix uninitialized use of variables.

[Carlos O'Donell <carlos@codesourcery.com>]

On Tue, Oct 23, 2007 at 04:20:12PM -0700, Jim Blandy wrote:
> 
> Carlos O'Donell <carlos at codesourcery.com> 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