From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16480 invoked by alias); 18 Dec 2012 06:17:47 -0000 Received: (qmail 16470 invoked by uid 22791); 18 Dec 2012 06:17:45 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_NO 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, 18 Dec 2012 06:17:41 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id D06C92E21E; Tue, 18 Dec 2012 01:17:40 -0500 (EST) 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 YrS0E5yNfP+r; Tue, 18 Dec 2012 01:17:40 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 652332E217; Tue, 18 Dec 2012 01:17:40 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 4F4DDC2D70; Tue, 18 Dec 2012 10:17:30 +0400 (RET) Date: Tue, 18 Dec 2012 06:17:00 -0000 From: Joel Brobecker To: Yao Qi Cc: gdb-patches@sourceware.org Subject: Re: [RFA/commit] Port GDB to powerpc-lynx178. Message-ID: <20121218061730.GE3273@adacore.com> References: <1355764695-5013-1-git-send-email-brobecker@adacore.com> <50CFF750.2090905@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <50CFF750.2090905@codesourcery.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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: 2012-12/txt/msg00611.txt.bz2 Yao, Thanks for your comments. Just one request: Would you mind cutting out the quoted test that you are not replying to? It makes no sense to see emails where you have to scroll pages and pages before seeing a one-line comment. > >+ /* Stack pointer must be quadword aligned. */ > >+ sp &= -16; > > How about 'sp = align_down (sp, 16);'? [...] > >+ /* Add location required for the rest of the parameters. */ > >+ space = (space + 15) & -16; > > How about 'space = align_up (space, 16);'? and we may use > 'align_up (XXX, 4)' somewhere else in this patch. Why not indeed. > >+ /* The only noticeable difference between Lynx178 XCOFF files and > >+ AIX XCOFF files comes from the fact that there are no shared > >+ libraries on Lynx178. So if the number of import files is > >+ different from zero, it cannot be a Lynx178 binary. */ > >+ if (xcoff_get_n_import_files (abfd) != 0) > >+ return GDB_OSABI_UNKNOWN; > > As your comments said, we need the function returning a flag > indicating 'the xcoff file has shared libraries or not', and looks > the precise number of import files doesn't matter here. I suggest > that rename function 'xcoff_get_n_import_files' to > 'xcoff_has_import_files'. While your suggestion may be good enough for today, there might come a day where someone will want the actual number of imports. Since it does not cost anything to provide that information, it would seem silly to spend any effort downgrading the function, and take the risk of having to undo those changes someday. Thanks, -- Joel