[PATCH] D21995: [ELF] Implement minimal PHDRS parser and section-to-segment assignment

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 12 07:39:22 PDT 2016


grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:286
@@ +285,3 @@
+  ssize_t RelroNum = -1;
+  Phdr *Load = nullptr;
+  uintX_t Flags = PF_R;
----------------
evgeny777 wrote:
> grimar wrote:
> > evgeny777 wrote:
> > > Well, ErrorOr<T> looks a way too heavy to represent valid/invalid array index, doesn't it?
> > > 
> > Right. That is why I suggested to use int32_t/uint32_t depending on what you need here. It is what we generally use in lld for such cases.
> > But we do not use int and never used ssize_t yet I think.
> There is plenty of int and plenty of size_t. Unfortunately I don't know any windows analog for 'grep -w', so just beleive me :)
> Yes, there isn't any ssize_t, right, but on the other hand - why not use it?
Well, I think there should be a reason for using something new, like new type in you code. What is the reason for you ?
We used other types all the way before, so at least there is a reason not to use ssize_t - and that readon is consistency. 
At the same time that is just my opinion here, I would not intoduce using of new type, but lets wait for another reviewers comments then.


http://reviews.llvm.org/D21995





More information about the llvm-commits mailing list