From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gdb-patches-return-60567-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Received: (qmail 3826 invoked by alias); 11 Dec 2008 23:05:14 -0000
Received: (qmail 3627 invoked by uid 22791); 11 Dec 2008 23:05:13 -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.43rc1) with ESMTP; Thu, 11 Dec 2008 23:04:32 +0000
Received: (qmail 608 invoked from network); 11 Dec 2008 23:04:30 -0000
Received: from unknown (HELO orlando.local) (pedro@127.0.0.2)   by mail.codesourcery.com with ESMTPA; 11 Dec 2008 23:04:30 -0000
From: Pedro Alves <pedro@codesourcery.com>
To: gdb-patches@sourceware.org,  tromey@redhat.com
Subject: Re: RFA: fix macro expansion bug
Date: Thu, 11 Dec 2008 23:05:00 -0000
User-Agent: KMail/1.9.10
References: <m3ljumcs2v.fsf@fleche.redhat.com>
In-Reply-To: <m3ljumcs2v.fsf@fleche.redhat.com>
MIME-Version: 1.0
Content-Type: text/plain;   charset="us-ascii"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Message-Id: <200812112304.39302.pedro@codesourcery.com>
X-IsSubscribed: yes
Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
X-SW-Source: 2008-12/txt/msg00213.txt.bz2

On Thursday 11 December 2008 21:36:24, Tom Tromey wrote:
> This patch fixes a macro expansion bug pointed out by Pedro.
> 
> The bug is that get_pp_number incorrectly handles pp-number tokens
> starting with '.'.  According to the C standard, it ought to only
> accept '.' followed by a digit at the beginning of a pp-number.

Indeed it does.  Clearly a bug.

> However, it unconditionally accepts a leading '.'.
> 
> Built and regtested on x86-64 (compile farm).
> Test case included.
> 
> Please review.
> 

Looks good, this is OK.

Though, I'm having a bit of trouble convincing myself that the logic to
handle 'pp-number e|E|p|P|. sign' below is 100% sane.

static int
get_pp_number (struct macro_buffer *tok, char *p, char *end)
{
...
      while (p < end)
        {
          if (macro_is_digit (*p)
              || macro_is_identifier_nondigit (*p)
              || *p == '.')
            p++;
          else if (p + 2 <= end
                   && strchr ("eEpP.", *p)
                   && (p[1] == '+' || p[1] == '-'))
            p += 2;
          else
            break;
        }

It seems macro_is_identifier_nondigit will always eat any of "eEpP",
thus, say, when parsing "1e-" only "1e" will be identified as a pp
number, leaving "+" in the stream.  Is this right?

> +
> +# Regression test for pp-number bug.
> +gdb_test "macro define si_addr fields.fault.si_addr" \
> +  "" \
> +  "define si_addr macro"
> +gdb_test "macro expand siginfo.si_addr" \
> +  "expands to: siginfo. fields.fault.si_addr" \
                          ^
Just curious, as it's just a visual annoyance: do you know where
this space comes from?  Do we store the definition with the space for
some reason?  We don't get that extra space if the define came
from the code, instead of from a 'macro define'.

-- 
Pedro Alves