From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gdb-patches-return-60988-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Received: (qmail 4595 invoked by alias); 8 Jan 2009 13:37:54 -0000
Received: (qmail 4585 invoked by uid 22791); 8 Jan 2009 13:37:54 -0000
X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 	tests=AWL,BAYES_00
X-Spam-Check-By: sourceware.org
Received: from outdoor.onevision.de (HELO outdoor.onevision.de) (212.77.172.51)     by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 08 Jan 2009 13:37:50 +0000
Received: from sanders.onevision.de (moonrace [212.77.172.62]) 	by outdoor.onevision.de (8.14.3/8.13.7/ROSCH/DDB) with ESMTP id n08DbgQB018108; 	Thu, 8 Jan 2009 14:37:47 +0100
In-Reply-To: <20090108130904.GD20220@adacore.com>
To: Joel Brobecker <brobecker@adacore.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [patch] Reading coff-pe-read files
MIME-Version: 1.0
Message-ID: <OF15D970B4.0B88A96E-ONC1257538.004ACDFB-C1257538.004ADC02@onevision.de>
From: Kai Tietz <Kai.Tietz@onevision.com>
Date: Thu, 08 Jan 2009 13:37:00 -0000
Content-Type: text/plain; charset="US-ASCII"
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: 2009-01/txt/msg00158.txt.bz2

Joel Brobecker <brobecker@adacore.com> wrote on 08.01.2009 14:09:04:

> > > > > 2009-01-08  Kai Tietz  <kai.tietz@onevision.com>
> > > > > 
> > > > >         * coff-pe-read.c (read_pe_exported_syms): Enable read of 
PE+ 
> > > > > export directory.
> > 
> > Approved.
> 
> Actually, the compiler just spotted a couple of syntax errors.
> GASP! We should have both been more careful.
> Can you please fix them before checking in?
> 
> > -  if ((strcmp (target, "pe-i386") != 0) && (strcmp (target, "pei-
> i386") != 0))
> > +  is_pe64 = ((strcmp (target, "pe-x86-64") == 0)
> > +         || ((strcmp (target, "pei-x86-64") == 0));
>                ^^ One '(' too many here.
> 
> Also, I can never determine in diffs whether the weird formatting
> is because of tabs or because it is wrong.  But in this case,
> the "||" is put at the wrong location. I know it looks nicer if
> the two strcmp expressions are aligned, but any formatter will
> destroy that, and we do use GNU indent once in a while.  So let's
> be consistent and format the above as follow:
> 
> > +  is_pe64 = ((strcmp (target, "pe-x86-64") == 0)
> > +             || ((strcmp (target, "pei-x86-64") == 0));
> 
> Same for is_pe32.
> 
> > +  if (is_pe64)
> > +    num_entries = pe_get32 (dll, opthdr_ofs + 108;
>                                                     ^^
>                                           missing ')' here.
> 
> Thanks,
> -- 
> Joel
> 

I corrected it in my patch.

Sorry,
Kai

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.