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

Frederic Riss friss at apple.com
Thu Dec 18 11:29:35 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);
----------------
samsonov wrote:
> 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.
Makes sense, I'll add the check.

================
Comment at: tools/dsymutil/BinaryHolder.cpp:78
@@ +77,3 @@
+BinaryHolder::MapArchiveAndGetMemberBuffer(StringRef Filename) {
+  StringRef ArchiveFilename = Filename.substr(0, Filename.find('('));
+
----------------
samsonov wrote:
> 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)
> 
Sorry for the confusion. Unfortunately, I think there's no nice way to handle this. The member could very well have an opening parenthesis in its name, making the last opening parenthesis the wrong one too. An option would be to try every parenthesis in the filename in turn, but I'd rather leave that out of this patch (not sure we ever want to handle it).

http://reviews.llvm.org/D6690

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






More information about the llvm-commits mailing list