From: Joel Brobecker <brobecker@adacore.com>
To: Yao Qi <yao@codesourcery.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [RFA/commit] Port GDB to powerpc-lynx178.
Date: Tue, 18 Dec 2012 06:17:00 -0000 [thread overview]
Message-ID: <20121218061730.GE3273@adacore.com> (raw)
In-Reply-To: <50CFF750.2090905@codesourcery.com>
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
next prev parent reply other threads:[~2012-12-18 6:17 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-17 17:18 Joel Brobecker
2012-12-17 18:41 ` Tom Tromey
2012-12-18 4:56 ` Yao Qi
2012-12-18 6:17 ` Joel Brobecker [this message]
2012-12-18 8:29 ` Yao Qi
2012-12-18 15:00 ` Joel Brobecker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20121218061730.GE3273@adacore.com \
--to=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=yao@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox