macro usage in bfd

[Alan Modra <amodra at bigpond dot net dot au>]

On Tue, May 25, 2004 at 10:15:52PM -0400, Ian Lance Taylor wrote:
> "H. J. Lu" <hjl@lucon.org> writes:
> > How about changing sec->name to bfd_get_section_name (bfd, sec)?
> I think that would be a good idea.

I don't agree.  I don't disagree that strongly, but..

pro (general):
o  Abstraction is generally a good idea.
o  Using a macro (or better, inline function) to replace a non-obvious
   expression with a macro that by its name documents its purpose helps
   to make code readable.  I strongly prefer appropriate names in the
   code over comments.

con (specific):
o  More typing.
o  Not more readable.  Longer lines, resulting in more line wraps.
o  No extra info conveyed by using the macro.  In some cases similar
   bfd macros convey *less* info, eg. bfd_get_section_alignment, which
   I recently replaced with sec->alignment_power in merge.c.  The latter
   hints that it is a power of 2 value, not a byte alignment.
o  The bfd param is unused, and in some places unavailable.  People
   write sec->owner just to satisfy the macro, but sec->owner is NULL in
   some cases.  Such code possibly won't work if we ever need to use
   the bfd in the macro.
o  Abstraction isn't alway appropriate in library code (as distinct from
   user code).  eg. bfd_set_section_vma sets lma too, behind your back.
   Another example is the mess we have with bfd_section_size,
   bfd_get_section_size_before_reloc, bfd_get_section_size_after_reloc.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre