From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8934 invoked by alias); 26 Oct 2007 15:39:29 -0000 Received: (qmail 8925 invoked by uid 22791); 26 Oct 2007 15:39:28 -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; Fri, 26 Oct 2007 15:39:25 +0000 Received: (qmail 17714 invoked from network); 26 Oct 2007 15:39:23 -0000 Received: from unknown (HELO localhost) (carlos@127.0.0.2) by mail.codesourcery.com with ESMTPA; 26 Oct 2007 15:39:23 -0000 Date: Fri, 26 Oct 2007 18:51: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: <20071026153920.GD21241@lios> References: <20071020172137.GC28823@lios> <20071024175743.GP21241@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/msg00718.txt.bz2 On Wed, Oct 24, 2007 at 11:20:44AM -0700, Jim Blandy wrote: > > In all truth I would have rather returned a status and let the caller > > determine the error message. > > I can see the appeal, but that sounds like a lot of work: > > - Change all the 'unpack_' functions to take the text pointer by > reference, update it in place, and return a status. > - Change all their callers to pass the text pointer by reference, > check the error code, and return an error status. > > And the win would be that you can say specifically what part of the > packet didn't parse, instead of just saying that the packet didn't > parse. It would be cleaner overall, but speaking for myself, there > are other things I'd appreciate more. :) Agreed. > The callers of the 'unpack_' functions currently seem to assume that > the callees will raise errors as needed; it doesn't seem too bad to go > along with that. Yes, that sounds good. Let's fix a real bug, and prevent a future compiler warning. OK to checkin? Cheers, Carlos. -- Carlos O'Donell CodeSourcery carlos@codesourcery.com (650) 331-3385 x716 gdb/ 2007-10-26 Carlos O'Donell * remote.c (unpack_nibble): error if buffer is not valid hex. * symtab.c (find_line_symtab): Initialize exact. Adjust comment to mention `no match' case. Index: remote.c =================================================================== RCS file: /cvs/src/src/gdb/remote.c,v retrieving revision 1.271 diff -u -p -r1.271 remote.c --- remote.c 8 Oct 2007 12:55:09 -0000 1.271 +++ remote.c 26 Oct 2007 15:38:13 -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 (_("Packet contains invalid hex data.")); return buf; } Index: symtab.c =================================================================== RCS file: /cvs/src/src/gdb/symtab.c,v retrieving revision 1.167 diff -u -p -r1.167 symtab.c --- symtab.c 24 Oct 2007 13:25:16 -0000 1.167 +++ symtab.c 26 Oct 2007 15:38:13 -0000 @@ -2252,7 +2252,7 @@ find_pc_line (CORE_ADDR pc, int notcurre struct symtab * find_line_symtab (struct symtab *symtab, int line, int *index, int *exact_match) { - int exact; + int exact = 0; /* BEST_INDEX and BEST_LINETABLE identify the smallest linenumber > LINE so far seen. */ @@ -2267,10 +2267,10 @@ find_line_symtab (struct symtab *symtab, best_index = find_line_common (best_linetable, line, &exact); if (best_index < 0 || !exact) { - /* Didn't find an exact match. So we better keep looking for - another symtab with the same name. In the case of xcoff, - multiple csects for one source file (produced by IBM's FORTRAN - compiler) produce multiple symtabs (this is unavoidable + /* Didn't find an exact match, or any match. So we better keep + looking for another symtab with the same name. In the case of + xcoff, multiple csects for one source file (produced by IBM's + FORTRAN compiler) produce multiple symtabs (this is unavoidable assuming csects can be at arbitrary places in memory and that the GLOBAL_BLOCK of a symtab has a begin and end address). */