Change interface for providing files to Minidump (#19). r=bryner

- Interface change: Minidump constructor now accepts a const string& path
   argument instead of int fd.  Minidump will open the file on first Read
   and close it upon destruction.
 - Adapt callers to new interface, no longer leaking file descriptors.

http://groups.google.com/group/airbag-dev/browse_thread/thread/ff24dbcde7db8ae3


git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@20 4c0a9323-5329-0410-9bdc-e9ce6186880e

src/processor/crash_report_processor.cc

@@ -12,9 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
 #include "google/crash_report.h"
 #include "google/crash_report_processor.h"
 #include "processor/minidump.h"
@@ -31,12 +28,7 @@ CrashReportProcessor::~CrashReportProcessor() {
 
 bool CrashReportProcessor::ProcessReport(CrashReport *report,
                                          const string &minidump_file) {
-  int fd = open(minidump_file.c_str(), O_RDONLY);
+  Minidump dump(minidump_file);
-  if (fd == -1) {
-    return false;
-  }
-
-  Minidump dump(fd);
   if (!dump.Read()) {
     return false;
   }

src/processor/minidump.cc

@@ -19,14 +19,18 @@
 // Author: Mark Mentovai
 
 
+#include <fcntl.h>
 #include <stdio.h>
 #include <time.h>
 #include <unistd.h>
 #ifdef _WIN32
 #include <io.h>
 typedef SSIZE_T ssize_t;
+#define open _open
 #define read _read
 #define lseek _lseek
+#else // _WIN32
+#define O_BINARY 0
 #endif // _WIN32
 
 #include <map>
@@ -1693,11 +1697,12 @@ void MinidumpMiscInfo::Print() {
 //
 
 
-Minidump::Minidump(int fd)
+Minidump::Minidump(const string& path)
     : header_(),
       directory_(NULL),
       stream_map_(NULL),
-      fd_(fd),
+      path_(path),
+      fd_(-1),
       swap_(false),
       valid_(false) {
 }
@@ -1706,6 +1711,25 @@ Minidump::Minidump(int fd)
 Minidump::~Minidump() {
   delete directory_;
   delete stream_map_;
+  if (fd_ != -1)
+    close(fd_);
+}
+
+
+bool Minidump::Open() {
+  if (fd_ != -1) {
+    // The file is already open.  Seek to the beginning, which is the position
+    // the file would be at if it were opened anew.
+    return SeekSet(0);
+  }
+
+  // O_BINARY is useful (and defined) on Windows.  On other platforms, it's
+  // useless, and because it's defined as 0 above, harmless.
+  fd_ = open(path_.c_str(), O_RDONLY | O_BINARY);
+  if (fd_ == -1)
+    return false;
+
+  return true;
 }
 
 
@@ -1718,6 +1742,9 @@ bool Minidump::Read() {
 
   valid_ = false;
 
+  if (!Open())
+    return false;
+
   if (!ReadBytes(&header_, sizeof(MDRawHeader)))
     return false;
 

src/processor/minidump.h

@@ -561,10 +561,8 @@ class MinidumpMiscInfo : public MinidumpStream {
 // and provides access to the minidump's top-level stream directory.
 class Minidump {
  public:
-  // fd is a randomly seekable file descriptor that is open and is
+  // path is the pathname of a file containing the minidump.
-  // positioned at the beginning of the MDRawHeader structure (byte offset
+  Minidump(const string& path);
-  // 0).
-  Minidump(int fd);
 
   ~Minidump();
 
@@ -652,6 +650,9 @@ class Minidump {
 
   template<typename T> T* GetStream(T** stream);
 
+  // Opens the minidump file, or if already open, seeks to the beginning.
+  bool Open();
+
   MDRawHeader               header_;
 
   // The list of streams.
@@ -660,7 +661,11 @@ class Minidump {
   // Access to streams using the stream type as the key.
   MinidumpStreamMap*        stream_map_;
 
+  // The pathname of the minidump file to process, set in the constructor.
+  const string              path_;
+
   // The file descriptor for all file I/O.  Used by ReadBytes and SeekSet.
+  // Set based on the |path_| member by Open, which is called by Read.
   int                       fd_;
 
   // swap_ is true if the minidump file should be byte-swapped.  If the

src/processor/minidump_dump.cc

@@ -17,22 +17,15 @@
 //
 // Author: Mark Mentovai
 
-#include <errno.h>
-#include <fcntl.h>
 #include <stdlib.h>
 #include <stdio.h>
-#include <string.h>
+
-#ifndef _WIN32
+#include <string>
-#include <unistd.h>
-#define O_BINARY 0
-#else // !_WIN32
-#include <io.h>
-#define open _open
-#endif // !_WIN32
 
 #include "processor/minidump.h"
 
 
+using std::string;
 using namespace google_airbag;
 
 
@@ -42,13 +35,7 @@ int main(int argc, char** argv) {
     exit(1);
   }
 
-  int fd = open(argv[1], O_RDONLY | O_BINARY);
+  Minidump minidump(argv[1]);
-  if (fd == -1) {
-    printf("open failed\n");
-    exit(1);
-  }
-
-  Minidump minidump(fd);
   if (!minidump.Read()) {
     printf("minidump.Read() failed\n");
     exit(1);

src/processor/minidump_stackwalk.cc

@@ -17,23 +17,16 @@
 //
 // Author: Mark Mentovai
 
-#include <errno.h>
-#include <fcntl.h>
 #include <stdlib.h>
 #include <stdio.h>
-#include <string.h>
+
-#ifndef _WIN32
+#include <string>
-#include <unistd.h>
-#define O_BINARY 0
-#else // !_WIN32
-#include <io.h>
-#define open _open
-#endif // !_WIN32
 
 #include "processor/minidump.h"
 #include "processor/stackwalker_x86.h"
 
 
+using std::string;
 using namespace google_airbag;
 
 
@@ -43,13 +36,7 @@ int main(int argc, char** argv) {
     exit(1);
   }
 
-  int fd = open(argv[1], O_RDONLY | O_BINARY);
+  Minidump minidump(argv[1]);
-  if (fd == -1) {
-    fprintf(stderr, "open failed\n");
-    exit(1);
-  }
-
-  Minidump minidump(fd);
   if (!minidump.Read()) {
     fprintf(stderr, "minidump.Read() failed\n");
     exit(1);