This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] backends: Check type DIE exists before calling dwarf_tag ().


Please don't use implicit boolean coercion, i.e. "!ptr".  Use "ptr == NULL".

Macros that return are always pretty nonobvious.  This one is used in few
enough places that I suppose it's not going to be too overly confusing.
But it's still not the best thing to do if there is anything else not too
clunky you can choose instead.  At least give a name that makes clear that
it might return.

Unconditional use of (older) GCC extensions in elfutils code is fine.
So you could make it:

#define DWARF_TAG_OR_RETURN(die) \
  ({ if ((die) == NULL) return -1; dwarf_tag (die); })

and then the uses are a more concise and natural looking:

	int tag = DWARF_TAG_OR_RETURN (typedie);

And any macro that evaluates its argument more than once should at least
have a comment mentioning that it does so.  Using a temporary to avoid that
is not really necessary for something like this with a small set of users,
but it never hurts:

#define DWARF_TAG_OR_RETURN(die) \
  ({ Dwarf_Die *_die = (die); \
     if (_die == NULL) return -1; \
     dwarf_tag (_die); })


Thanks,
Roland

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]