From: Pedro Alves <palves@redhat.com>
To: Sergio Durigan Junior <sergiodj@redhat.com>,
binutils@sourceware.org, gdb-patches@sourceware.org
Subject: Re: [RFC/PATCH] Fix `bfd_{get,set}_*' macros
Date: Thu, 03 May 2012 16:01:00 -0000 [thread overview]
Message-ID: <4FA2ABAD.3010403@redhat.com> (raw)
In-Reply-To: <20120503075305.GJ635@bubble.grove.modra.org>
On 05/03/2012 08:53 AM, Alan Modra wrote:
> On Thu, May 03, 2012 at 01:19:21AM -0300, Sergio Durigan Junior wrote:
>> * bfd-in2.h (bfd_get_section_name, bfd_get_section_vma,
>> bfd_get_section_lma, bfd_get_section_alignment, bfd_section_name,
>> bfd_section_size, bfd_get_section_flags,
>> bfd_get_section_userdata): Rewrite macros in order to use the
>> `bfd' argument.
>> * elf-vxworks.c (elf_vxworks_finish_dynamic_entry): Pass proper `bfd'
>> as the first argument for `bfd_get_section_alignment'.
>> * elf32-arm.c (create_ifunc_sections): Likewise, for
>> `bfd_set_section_alignment'.
>
> The above is OK. You guessed wrongly for all the bfd args below,
> except in elf64-ppc.c. I suggest expanding the macros instead. I've
> noted the correct bfds below but IMO these macros serve no useful
> purpose.
#define bfd_get_section_name(bfd, ptr) ((ptr)->name + 0)
#define bfd_get_section_vma(bfd, ptr) ((ptr)->vma + 0)
#define bfd_get_section_lma(bfd, ptr) ((ptr)->lma + 0)
#define bfd_get_section_alignment(bfd, ptr) ((ptr)->alignment_power + 0)
#define bfd_section_name(bfd, ptr) ((ptr)->name)
#define bfd_section_size(bfd, ptr) ((ptr)->size)
#define bfd_get_section_size(ptr) ((ptr)->size)
#define bfd_section_vma(bfd, ptr) ((ptr)->vma)
#define bfd_section_lma(bfd, ptr) ((ptr)->lma)
#define bfd_section_alignment(bfd, ptr) ((ptr)->alignment_power)
#define bfd_get_section_flags(bfd, ptr) ((ptr)->flags + 0)
#define bfd_get_section_userdata(bfd, ptr) ((ptr)->userdata)
Or replace with bfd_section_name, bfd_section_vma, etc. (making them
lvalues along the way) ?
If dropping the bfd argument, IMO, it's better to use the 'bfd_section'
prefix for bfd_section macros (like bfd_section_name, etc. above), rather
than 'bfd_get_', and leave the latter for macros/functions that really
take a struct bfd as "this". That'd mean
s/bfd_get_section_flags/bfd_section_flags/ for example.
But IMO, we shouldn't require Sérgio do this, unless he
wants to. His patch just fixes a handful of places to correctly
use the already years-old existing interface...
--
Pedro Alves
next prev parent reply other threads:[~2012-05-03 16:01 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-03 4:19 Sergio Durigan Junior
2012-05-03 7:54 ` Alan Modra
2012-05-03 16:01 ` Pedro Alves [this message]
2012-05-03 16:22 ` Sergio Durigan Junior
2012-05-04 21:46 ` Sergio Durigan Junior
2012-05-10 1:54 ` Sergio Durigan Junior
2012-05-16 18:28 ` nick clifton
2012-05-16 19:48 ` Sergio Durigan Junior
2012-05-03 13:17 ` Doug Evans
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=4FA2ABAD.3010403@redhat.com \
--to=palves@redhat.com \
--cc=binutils@sourceware.org \
--cc=gdb-patches@sourceware.org \
--cc=sergiodj@redhat.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