[PATCH] D93096: [llvm-readelf] - Improve ELF type field dumping.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D93096#2451814 <https://reviews.llvm.org/D93096#2451814>, @grimar wrote:

> I guess it might be OK. It is probably a bit inconistent with GNU which does:
>
>   static char *
>   get_file_type (unsigned e_type)
>   {
>     static char buff[32];
>   
>     switch (e_type)
>       {
>       case ET_NONE: return _("NONE (None)");
>       case ET_REL:  return _("REL (Relocatable file)");
>       case ET_EXEC: return _("EXEC (Executable file)");
>       case ET_DYN:  return _("DYN (Shared object file)");
>       case ET_CORE: return _("CORE (Core file)");
>   
>       default:
>         if ((e_type >= ET_LOPROC) && (e_type <= ET_HIPROC))
>   	snprintf (buff, sizeof (buff), _("Processor Specific: (%x)"), e_type);
>         else if ((e_type >= ET_LOOS) && (e_type <= ET_HIOS))
>   	snprintf (buff, sizeof (buff), _("OS Specific: (%x)"), e_type);
>         else
>   	snprintf (buff, sizeof (buff), _("<unknown>: %x"), e_type);
>         return buff;
>       }
>   }

I'm not sure it is all that consistent actually - in the GNU case, the code immediately returns the print out of known types before falling back to unknown types, so if we had the same patch in GNU readelf, it'd likely be the same as the old output.

> But at the same time it looks reasonable to print known/named types first.
> I can prepare a patch, though seems there is no way to test such change.
>
> Lets see what @MaskRay and/or others think too.

Thanks for considering it. FWIW, I agree testing might be a possible issue, but on the other hand, I think it could have easily happened that someone wrote my suggestion in the first place, and I don't think I'd have noticed any real lack of testing.

Also a very minor advantage of my approach is that if llvm-readelf were to ever gain support for known OS/Processor specific types, the code would be simpler (just add to the list), and the output possibly more consistent.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93096/new/

https://reviews.llvm.org/D93096