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

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 14 05:56:27 PST 2020


jhenderson added a comment.

We've just run into an unexpected side-effect of this change, when this got merged into our downstream branch. We have a number of new ET_* values in the OS-specific range. In our downstream branch, we've added new entries to `ElfObjectFileType` so that these values are printed nicely as `Type: OurCustomType`. However, with this change, this has changed to `Type: OS Specific: (OurCustomType)` which isn't ideal. We can probably live with it, but I wonder if a small refactor might be better to allow us to have a clean downstream patch without really harming upstream code. My initial suggestion would be to change to something like this:

    if (!makeArrayRef(ElfObjectFileType).end() ==
               llvm::find_if(ElfObjectFileType,
                             [&](const EnumEntry<unsigned> &E) {
                               return E.Value == e.e_type;
                             })) {
      if (e.e_type >= ET_LOPROC) {
        Str = "Processor Specific: (" + Str + ")";
      } else if (e.e_type >= ET_LOOS) {
        Str = "OS Specific: (" + Str + ")";
      } else {
        Str = "<unknown>: " + Str;
      }
  }

I think the overall code complexity is about the same. What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93096



More information about the llvm-commits mailing list