[PATCH] D28561: [ELF] - Move the addition of synthetics from addPredefinedSections()

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 12 00:39:38 PST 2017


grimar added inline comments.


================
Comment at: ELF/Writer.cpp:1598-1600
+  EHdr->e_shstrndx = In<ELFT>::ShStrTab->OutSec
+                         ? In<ELFT>::ShStrTab->OutSec->SectionIndex
+                         : SHN_UNDEF;
----------------
ruiu wrote:
> I'd think this is slightly more readable.
> 
>   if (In<ELFT>::ShStrTab->OutSec)
>     EHdr->e_shstrndx = In<ELFT>::ShStrTab->OutSec->SectionIndex;
May be. At least it shorter. Though this form does not explicitly assign SHN_UNDEF, it is a luck that SHN_UNDEF == 0.
I usually prefer not to assume values for named constants, even for such cases.

I'll use your suggestion to land this patch after resolving D28560 dependency as its seems a matter of taste in this place.


https://reviews.llvm.org/D28561





More information about the llvm-commits mailing list