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

[Alexey Samsonov]

Looks mostly good.


================
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);
----------------
You should check that Filename is long enough.

================
Comment at: tools/dsymutil/BinaryHolder.cpp:65
@@ +64,3 @@
+
+  for (const auto &Child : CurrentArchive->children())
+    if (auto NameOrErr = Child.getName())
----------------
This loop occupies 6 lines. Please add {} around its body.

================
Comment at: tools/dsymutil/BinaryHolder.cpp:78
@@ +77,3 @@
+BinaryHolder::MapArchiveAndGetMemberBuffer(StringRef Filename) {
+  StringRef ArchiveFilename = Filename.substr(0, Filename.find('('));
+
----------------
Hm, don't you need to take the last opening parenthesis?

================
Comment at: tools/dsymutil/BinaryHolder.h:84
@@ +83,3 @@
+      return *Derived;
+    return make_error_code(errc::executable_format_error);
+  }
----------------
Can we use smth. like object_error::invaild_file_type here?

================
Comment at: tools/dsymutil/BinaryHolder.h:90
@@ +89,3 @@
+  /// before calling this.
+  const object::ObjectFile &Get() { return *CurrentObjectFile; }
+
----------------
You can add
  assert(CurrentObjectFile);
here to make this code more self-documented.

================
Comment at: tools/dsymutil/BinaryHolder.h:93
@@ +92,3 @@
+  /// \brief Access to a derived version of the currently owned
+  /// ObjectFile. The conversion must have to be knwo to be valid.
+  template <typename ObjectFileType> const ObjectFileType &GetAs() {
----------------
s/must have to be knwo/must be known

http://reviews.llvm.org/D6690

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