[PATCH] [lld][PECOFF] Read relocation entries.

Sean Silva silvas at purdue.edu
Thu Jun 13 16:49:55 PDT 2013



================
Comment at: lib/ReaderWriter/PECOFF/WriterPECOFF.cpp:274-275
@@ +273,4 @@
+    llvm::object::coff_section header;
+    std::memset(header.Name, 0, sizeof(header.Name));
+    std::strncpy(header.Name, sectionName.data(), sizeof(header.Name));
+    header.VirtualSize = 0;
----------------
Try to avoid using unsafe C string manipulation like strncpy. If you really need to use strncpy, please add assertions that ensure the bounds are correct. For example something like `assert(sizeof(header.Name) < sectionName.size())`. Also, is the case where strncpy does not add the trailing NUL acceptable in this situation? Is sectionName NUL terminated?

================
Comment at: lib/ReaderWriter/PECOFF/ReaderCOFF.cpp:433
@@ +432,3 @@
+
+  /// Atomize defined symbols.
+  error_code AtomizeDefinedSymbols(SectionToSymbolsT &definedSymbols,
----------------
This comment doesn't help. Just delete it (or improve it).

================
Comment at: lib/ReaderWriter/PECOFF/ReaderCOFF.cpp:448
@@ +447,3 @@
+
+      for (COFFDefinedAtom* atom : atoms) {
+        if (!atom->name().empty())
----------------
`*` on the right. I recommend using clang-format.


http://llvm-reviews.chandlerc.com/D976



More information about the llvm-commits mailing list