From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22788 invoked by alias); 23 Mar 2010 18:53:24 -0000 Received: (qmail 22777 invoked by uid 22791); 23 Mar 2010 18:53:22 -0000 X-SWARE-Spam-Status: No, hits=0.3 required=5.0 tests=AWL,BAYES_00,KAM_STOCKTIP X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 23 Mar 2010 18:53:18 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id A90C42BAB7C; Tue, 23 Mar 2010 14:53:16 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id nWGWaLL+dIei; Tue, 23 Mar 2010 14:53:16 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 6E2B22BAB2C; Tue, 23 Mar 2010 14:53:16 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 82ABEF593B; Tue, 23 Mar 2010 11:53:14 -0700 (PDT) Date: Tue, 23 Mar 2010 18:53:00 -0000 From: Joel Brobecker To: Pierre Muller Cc: gdb-patches@sourceware.org Subject: Re: [RFC] Support for const char and strings in stabs reader Message-ID: <20100323185314.GA2882@adacore.com> References: <000201cac54d$246dcdd0$6d496970$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <000201cac54d$246dcdd0$6d496970$@muller@ics-cnrs.unistra.fr> User-Agent: Mutt/1.5.20 (2009-06-14) 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: 2010-03/txt/msg00795.txt.bz2 > 2010-03-16 Pierre Muller > > * stabsread.c (define_symbol): Add support for char > and string constants. As a general comment, the formatting of the section supporting strings is not consistent. It looks like it's because you have a mixture of spaces and tabs... > + case 's': > + { > + struct type *range_type; > + char quote = *p++; > + char *startp = p; > + gdb_byte *string_value; Can you add an empty line after your local variable declarations? > + if (quote != '\'' && quote != '"') > + { > + SYMBOL_CLASS (sym) = LOC_CONST; > + SYMBOL_TYPE (sym) = error_type (&p, objfile); > + SYMBOL_DOMAIN (sym) = VAR_DOMAIN; > + add_symbol_to_list (sym, &file_symbols); > + return sym; > + } > + /* Find matching quote, rejecting escaped quotes. */ > + while (*p && *p != quote) > + { > + if (*p == '\\') > + p++; > + if (*p) > + p++; > + } What if you don't find the matching quote? I think we should reject the stabs the same way you do when you don't find an opening quote. > + *p = '\0'; Outch. I see that you restore the stabs later on, but I am still concerned by this, particularly if any of the functions you use end up raising an exception... It looks like you do this mostly to be able to get the length of the string itself. But that's easy to obtain, thanks to the loop that searches the ending quote character. > + strcpy ((char *)string_value, startp); > + *p = quote; One question I asked myself, and got confused over, is: What happens with escaped quotes. For instance, let's imagine that we have the following string: 'hello \' there'. I think that the string value that you want to record is <>, not <>. In other word, the symbol value should not contain the backslash, right? Does it look like your current implementation will in fact contain it? > + range_type = create_range_type (NULL, > + objfile_type (objfile)->builtin_int, > + 0, strlen(startp)); The indentation is wrong, it should be: range_type = create_range_type (NULL, objfile_type (objfile)->builtin_int, 0, strlen(startp)); > + gdb_test "p /x constchar" " = 0x46" "char constant" > + gdb_test "p constString1" " = \"String1\"" "String constant 1" > + gdb_test "p constString2" " = \"String2\"" "String constant 2" > + gdb_test "p constString3" " = \"String3 with embedded quote ' in > the middle\"" "String constant 3" Your last test seems to contradict me on the escape character, but how does this end up working? -- Joel