[PATCH] D83482: [yaml2obj] - Add a syntax to override e_phoff, e_phentsize and e_phnum fields.

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.





================
Comment at: llvm/lib/ObjectYAML/ELFYAML.cpp:873-875
+  IO.mapOptional("PhOff", FileHdr.PhOff);
+  IO.mapOptional("PhEntSize", FileHdr.PhEntSize);
+  IO.mapOptional("PhNum", FileHdr.PhNum);
----------------
grimar wrote:
> jhenderson wrote:
> > The section header table related fields use "SH" not "Sh". It probably makes sense for us to standardise on one form or the other. In other words, I think we should probably use "PH*" here.
> Fields in `Sections` class also use `Sh*`:
> 
> ```
>   // This can be used to override the offset stored in the sh_name field.
>   // It does not affect the name stored in the string table.
>   Optional<llvm::yaml::Hex64> ShName;
> 
>   // This can be used to override the sh_offset field. It does not place the
>   // section data at the offset specified.
>   Optional<llvm::yaml::Hex64> ShOffset;
> 
>   // This can be used to override the sh_size field. It does not affect the
>   // content written.
>   Optional<llvm::yaml::Hex64> ShSize;
> 
>   // This can be used to override the sh_flags field.
>   Optional<llvm::yaml::Hex64> ShFlags;
> ```
> 
> So we have 2 options it seems:
> 1) Use "SH*" + "PH*" everywhere (i.e make the renaming you suggest + rename fields in `Sections`).
> 2) Rename "PH*" -> "Ph*" in a follow-up here (I assumed this action when wrote this patch).
> 
> What should we do?
>>! In D83482#2147128, @jhenderson wrote:
> Relatedly, how about we actually name these fields more precisely like their spec names, e.g. EShoff, EPhoff, EShentsize (or something equivalent).

But we do not add `E` prefix to other fields. E.g: we call `e_type` as `Type`. 
I renamed them to `PH*` (e.g. `PhEntSize` -> `PHEntSize`), because `phentsize` is Program **H**eader Entry Size, so it makes sense I think.


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

https://reviews.llvm.org/D83482