Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
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


  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