[PATCH] PE/COFF: rework how we handle base relocations

Rui Ueyama ruiu at google.com
Fri Jan 16 14:09:47 PST 2015


LGTM with these fixes.


REPOSITORY
  rL LLVM

================
Comment at: lib/ReaderWriter/PECOFF/WriterPECOFF.cpp:769
@@ -762,3 +768,3 @@
   // can inspect relocations, and fix the tests (base-reloc.test, maybe
   // others) to use those messages.
 
----------------
Not directly related to this change, but can you remove this TODO? It's obsolete.

================
Comment at: lib/ReaderWriter/PECOFF/WriterPECOFF.cpp:785
@@ +784,3 @@
+        case llvm::COFF::IMAGE_REL_I386_DIR32:
+          reloc = std::make_pair(address, llvm::COFF::IMAGE_REL_BASED_HIGHLOW);
+          break;
----------------
I'd do like this because this switch has only one non-default case.

  if (ref->kindValue() == llvm::COFF::IMAGE_REL_I386_DIR32)
    reloc = std::make_pair(...);
  break;

================
Comment at: lib/ReaderWriter/PECOFF/WriterPECOFF.cpp:795
@@ +794,3 @@
+          break;
+        }
+        break;
----------------
ditto

================
Comment at: lib/ReaderWriter/PECOFF/WriterPECOFF.cpp:799
@@ +798,3 @@
+        switch (ref->kindValue()) {
+        default: continue;
+        case llvm::COFF::IMAGE_REL_ARM_ADDR32:
----------------
In other switches default is after all cases. (But my personal preference is if () {...} else if () {...} break.)

http://reviews.llvm.org/D7022

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list