[PATCH] D21556: [ELF] - Implemented version script hierarchies.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 22 01:58:40 PDT 2016


grimar added inline comments.

================
Comment at: ELF/OutputSections.cpp:1478
@@ -1473,1 +1477,3 @@
 
+unsigned getVersionNameStrTabOffset(StringRef Name) {
+  for (Version &V : Config->SymbolVersions)
----------------
ruiu wrote:
> `static size_t`?
Right..

================
Comment at: ELF/OutputSections.cpp:1510
@@ +1509,3 @@
+
+  Verdaux[-1].vda_next = sizeof(Elf_Verdaux);
+  Verdaux->vda_name = getVersionNameStrTabOffset(DependencyName);
----------------
davide wrote:
> It could be just me, but I'd rather prefer this -1 to be assigned to a variable or adding a comment to explain what this code does.
Ok, added comment. It is in common in versioning relative methods around here, btw.

================
Comment at: ELF/SymbolListFile.cpp:97
@@ -96,1 +96,3 @@
   expect("}");
+  if (Version.empty() || peek() == ";") {
+    expect(";");
----------------
ruiu wrote:
> I'd do
> 
>   if (!Version.empty() && peek() != ";")
>     Config->SymbolVersions.back().Dependency = next();
>   expect(";");
Done.


http://reviews.llvm.org/D21556





More information about the llvm-commits mailing list