[PATCH] [dsymutil] Implement the BinaryHolder object and gain archive support.

Frederic Riss friss at apple.com
Thu Dec 18 09:10:31 PST 2014


Thanks, will resubmit with you feedback. Some comments:


================
Comment at: tools/dsymutil/BinaryHolder.cpp:59
@@ +58,3 @@
+  if (!Filename.startswith(CurArchiveName) ||
+      Filename[CurArchiveName.size()] != '(')
+    return make_error_code(errc::no_such_file_or_directory);
----------------
samsonov wrote:
> You should check that Filename is long enough.
If Filename startsWith CurArchiveName, then FileName[CurArchiveName.size()] is valid because these StringRefs come from std::strings that are null terminated.

================
Comment at: tools/dsymutil/BinaryHolder.cpp:78
@@ +77,3 @@
+BinaryHolder::MapArchiveAndGetMemberBuffer(StringRef Filename) {
+  StringRef ArchiveFilename = Filename.substr(0, Filename.find('('));
+
----------------
samsonov wrote:
> Hm, don't you need to take the last opening parenthesis?
Not here, we are extracting the archive name. When we extract the member name in GetArchiveMemberBuffer we drop the last character.

http://reviews.llvm.org/D6690

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






More information about the llvm-commits mailing list