Create ScopedTempDir, implement it on POSIX, and use it where appropriate.

util/test/scoped_temp_dir_posix.cc

Index: util/test/scoped_temp_dir_posix.cc
diff --git a/util/test/scoped_temp_dir_posix.cc b/util/test/scoped_temp_dir_posix.cc
new file mode 100644
index 0000000000000000000000000000000000000000..dfc31f4e68daeb9d89c463cd4e65120efb76caf7
--- /dev/null
+++ b/util/test/scoped_temp_dir_posix.cc
@@ -0,0 +1,61 @@
+// Copyright 2014 The Crashpad Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "util/test/scoped_temp_dir.h"
+
+#include <dirent.h>
+#include <unistd.h>
+
+#include "base/logging.h"
+#include "gtest/gtest.h"
+#include "util/test/errors.h"
+
+namespace crashpad {
+namespace test {
+
+base::FilePath ScopedTempDir::CreateTemporaryDirectory() {
+  char dir_tempalate[] = "/tmp/com.googlecode.crashpad.temp_dir.XXXXXX";

[Mark Mentovai, 2014/12/31 05:51:09]

Test-specific things so far have used com.googlecode.crashpad.test as the
prefix.

[Robert Sesek, 2015/01/02 17:34:08]

Done.

+  CHECK(mkdtemp(dir_tempalate)) << ErrnoMessage("mkdtemp");

[Mark Mentovai, 2014/12/31 05:51:09]

Also: add << " " << dir_template?

[Mark Mentovai, 2014/12/31 05:51:09]

PCHECK(), not CHECK(). Then you can lose the ErrnoMessage and just have <<
"mkdtemp".

[Robert Sesek, 2015/01/02 17:34:08]

Done.

[Robert Sesek, 2015/01/02 17:34:09]

Done.

+  return base::FilePath(dir_tempalate);
+}
+
+void ScopedTempDir::RecursivelyDeleteTemporaryDirectory(
+    const base::FilePath& path) {

[Mark Mentovai, 2014/12/31 05:51:09]

dir_path might further disambiguate from entry_path (and adjust the .h too,
then).

+  DIR* dir = opendir(path.value().c_str());

[Mark Mentovai, 2014/12/31 05:51:10]

You ought to be able to make a scoper for this out of a ScopedGeneric really
easily. It can be local to this file for now.

[Robert Sesek, 2015/01/02 17:34:09]

I tried this, but it's more code and I don't think it helps in any significant
way.

+  ASSERT_TRUE(dir) << ErrnoMessage("opendir") << " " << path.value();
+
+  struct dirent* entry;

[Mark Mentovai, 2014/12/31 05:51:10]

Can you just call this dirent* without the struct tag as we do elsewhere in
Crashpad C++?

[Robert Sesek, 2015/01/02 17:34:08]

Done, though my preference is to keep "struct" when referencing C structures
from C++.

+  while ((entry = readdir(dir))) {
+    base::FilePath entry_path = path.Append(entry->d_name);

[Mark Mentovai, 2014/12/31 05:51:09]

No point in doing this until after you’ve checked for "." and "..".

[Robert Sesek, 2015/01/02 17:34:09]

Done.

+    if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) {

[Mark Mentovai, 2014/12/31 05:51:09]

#include <string.h>.

[Robert Sesek, 2015/01/02 17:34:08]

Done.

+      continue;
+    }
+
+    if (entry->d_type == DT_DIR) {
+      RecursivelyDeleteTemporaryDirectory(entry_path);

[Mark Mentovai, 2014/12/31 06:07:28]

You should set entry to nullptr to affirmatively state that it may be unusable
once you make this call. That might also be worth an inline comment.

The other alternative is to use readdir_r() here, but I don’t think that’s
necessary at all.

The code you wrote isn’t wrong in any way, but this is a readdir() gotcha and
it’s good to protect against someone in the future trying to use entry after
it’s become bogus. (The compiler will optimize the nullptr assignment into
oblivion.)

[Mark Mentovai, 2015/01/01 06:40:20]

I wrote:
Disregard per post-ball-drop discussion.

+    } else {
+      EXPECT_EQ(0, unlink(entry_path.value().c_str())) << ErrnoMessage("unlink")

[Mark Mentovai, 2014/12/31 05:51:09]

The message would be easier to read if you put the entire thing on a single
continuation line. Same for the closedir() and rmdir().

[Robert Sesek, 2015/01/02 17:34:08]

Done.

+                                                       << " "
+                                                       << entry_path.value();
+    }
+  }
+
+  EXPECT_EQ(0, closedir(dir)) << ErrnoMessage("closedir") << " "
+                              << path.value();
+  EXPECT_EQ(0, rmdir(path.value().c_str())) << ErrnoMessage("rmdir")
+                                            << " " << path.value();
+}
+
+}  // namespace test
+}  // namespace crashpad