Re: [PATCH 1/5] Rewrite DWARF string functions using known-dwarf macros.

[Roland McGrath <roland at hack dot frob dot com>]

> +2012-07-27  Mark Wielaard  <mjw@redhat.com>

Use current dates in the log entries.  Note also that if you use 'git
commit --amend' it preserves the old commit date, which is not helpful.
If you pass --reset-author it resets the date.

> +	* readelf.c (dwarf_tag_name): Renamed from dwarf_tag_string.
> +	Uses new dwarf_tag_string or adds unknown_%#x.

No longer matches reality.

> +	(print_ops): Remove known[]. Use dwarf_locexpr_opcode_string.

Put local variable names in caps, and there is no need for pseudo-syntax
like [], just the name is fine.  Put two spaces between sentences.

> +#define ONE_KNOWN_DW_FORM_DESC(NAME, CODE, DESC) ONE_KNOWN_DW_FORM(NAME, CODE)

Space before paren in the macro use.

> +static const char *
> +dwarf_locexpr_opcode_string (unsigned int code)
> +{
> +  static const char *const known[] =
> +    {
> +      /* Normally we can't affort building huge table of 64K entries,
> +	 most of them zero, just because there are a couple defined
> +	 values at the far end.  In case of opcodes, it's OK.  */
> +#define ONE_KNOWN_DW_OP_DESC(NAME, CODE, DESC) ONE_KNOWN_DW_OP(NAME, CODE)

Here too.

> +  const char *ret = NULL;
> +  if (likely (code < sizeof (known) / sizeof (known[0])))
> +    ret = known[code];
> +
> +  return ret;

It's more natural to nix RET and just return in the if.

> +}
> +
> +
> +/* Reused by all dwarf_foo_name functions.  */
> +static char unknown_buf[16];

lo_user+4294967295\0 is 19, so make it at least 20.

> +static const char *
> +dwarf_tag_name (unsigned int tag)
> +{
> +  const char *result = dwarf_tag_string (tag);
> +  if (unlikely (result == NULL))
> +    {
> +      if (tag >= DW_TAG_lo_user && tag <= DW_TAG_hi_user)
> +	snprintf (unknown_buf, sizeof unknown_buf, "lo_user+%#x",
> +		  tag - DW_TAG_lo_user);
> +      else
> +	snprintf (unknown_buf, sizeof unknown_buf, "??? (%#x)", tag);
> +      result = unknown_buf;
> +    }
> +  return result;
> +}

This pattern is repeated a lot, so write a subroutine:

static const char *
string_or_unknown (const char *known,
		   unsigned int lo_user, unsigned int hi_user)

Then unknown_buf can be static inside that function.


I'm not too sure about the (existing) distinction between things that
always have the number printed as well as the symbolic form.  I'm inclined
to say we should make readelf output more uniform: symbolic name only, and
number only if unrecognized.  But that is a behavior change that is
technically orthogonal to your changes here.


Thanks,
Roland