[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