[PATCH] D78709: [llvm-readobj] - Simplify conditions used for printing segment mappings. NFCI.

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar added inline comments.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:4083
+
+  // No section with zero size must be at the start or at the end of PT_DYNAMIC.
   return ((Sec.sh_type == ELF::SHT_NOBITS) ||
----------------
grimar wrote:
> jhenderson wrote:
> > grimar wrote:
> > > jhenderson wrote:
> > > > This comment belongs with the above if, I think.
> > > I think no. It seems to be about 
> > > 
> > > "(Sec.sh_offset > Phdr.p_offset && Sec.sh_offset < Phdr.p_offset + Phdr.p_filesz)"
> > > 
> > > One of conditions to get here is to have "Sec.sh_size == 0".
> > This comment is to do with sections with zero size and PT_DYNAMIC, so the comment belongs with the zero size/PT_DYNAMIC check surely?
> See:
> 
> `if (Phdr.p_type != ELF::PT_DYNAMIC || Phdr.p_memsz == 0 || Sec.sh_size != 0)` above means that
> we can get here only when a segment is `PT_DYNAMIC` and section has zero size.
> 
> The comment says "No section with zero size must be at the start or at the end of PT_DYNAMIC"
> 
> It is related to the following condition:
> ```
> (Sec.sh_offset > Phdr.p_offset && Sec.sh_offset < Phdr.p_offset + Phdr.p_filesz)
> ```
> 
> Note that `>` and `<` are used here instead of `>=` and `<=`. I.e. it checks that the empty section is not at the
> start or at the end. Just like comment says.
> 
> Right?
I think I'll be able to improve comments in this method to avoid confusion.


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

https://reviews.llvm.org/D78709