[PATCH] D65633: [Object] Create MutableELFObject Class for Doing Mutations on ELFObjectFiles [Part 3]

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/unittests/Object/MutableELFObjectTest.cpp:420
+    Value:    0
+    Binding:  STB_GLOBAL)");
+
----------------
Not sure, but it might be interesting to add some dynamic symbols and show that iterating over those isn't affected by the regular table changes.


================
Comment at: llvm/unittests/Object/MutableELFObjectTest.cpp:435
+  EXPECT_EQ(*NameOrErr, "first");
+  MutableObject.removeSymbol(FirstSym);
+
----------------
I think it would be interesting to see what happens when you remove the second symbol too, leaving only the null entry. I also think it would be interesting to test what happens if you try to remove the null entry (I think this should probably be an error).


================
Comment at: llvm/unittests/Object/MutableELFObjectTest.cpp:496
+
+  MutableObject.removeSection(Iter);
+
----------------
Again, a test case for trying to remove the null section header might be interesting (it should probably be an error).


================
Comment at: llvm/unittests/Object/MutableELFObjectTest.cpp:506-507
+  ASSERT_FALSE(Iter->getName(Name));
+  EXPECT_NE(Name, ".remove_me");
+  EXPECT_NE(Name, ".remove_me_too");
+}
----------------
Rather than looking at just one, you should probably loop over all of the remaining sections, including the null one.


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

https://reviews.llvm.org/D65633