[PATCH] [lld][mach-o] binary reader and writer

Rui Ueyama ruiu at google.com
Tue Nov 5 16:26:20 PST 2013


  LGTM with minor comments.


================
Comment at: lib/ReaderWriter/MachO/MachONormalizedFileBinaryUtils.h:199
@@ +198,3 @@
+bitFieldExtract(uint32_t value, bool isBigEndianBigField, 
+                                        uint8_t firstBit, uint8_t bitCount) {
+  const uint32_t mask = ((1<<bitCount)-1); 
----------------
kledzik at apple.com wrote:
> Rui Ueyama wrote:
> > I think it's not in LLVM coding style. Can you run clang-format on this patch?
> I just tried running clang-format on this file and all it did was lose the columnar alignment of the right hand sides.
Then can you fix the layout by hand? As to this line, the second line of the parameter list should start at the same column as the first line. Same comments for the same errors.

================
Comment at: lib/ReaderWriter/MachO/MachONormalizedFileBinaryUtils.h:285
@@ +284,3 @@
+  memset(s, 0, 16);
+  memcpy(s, str.begin(), (str.size() > 16) ? 16: str.size());
+}
----------------
So if the string is longer than 16 characters, "s" will not be NUL-terminated. I don't have enough knowledge about MachO whether that's correct or not. Is this intended?

================
Comment at: lib/ReaderWriter/MachO/MachONormalizedFileBinaryWriter.cpp:215
@@ +214,3 @@
+  } 
+  while( more );
+}
----------------
Should be at the end of the last line.

================
Comment at: lib/ReaderWriter/MachO/MachONormalizedFileBinaryUtils.h:234
@@ +233,3 @@
+  }
+  else {
+    result.offset     = r0;
----------------
Rui Ueyama wrote:
> Ditto
There are other places with the same error. You can grep '^\s+else {' to find them.


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

BRANCH
  svn

ARCANIST PROJECT
  lld



More information about the llvm-commits mailing list