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

Alexey Samsonov vonosmas at gmail.com
Thu Dec 18 11:16:58 PST 2014


================
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);
----------------
friss wrote:
> 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.
Right, but if StringRef is a type of an argument in your public interface, it's better not to depend on the fact that these StringRef's will come from a particular source.

================
Comment at: tools/dsymutil/BinaryHolder.cpp:78
@@ +77,3 @@
+BinaryHolder::MapArchiveAndGetMemberBuffer(StringRef Filename) {
+  StringRef ArchiveFilename = Filename.substr(0, Filename.find('('));
+
----------------
friss wrote:
> 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.
I mean last *opening* parenthesis. Can we have a Filename:
  my(small)archive.a(member.o)

http://reviews.llvm.org/D6690

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






More information about the llvm-commits mailing list