Add wrappers around getxattr() and setxattr().

util/mac/xattr.cc

Index: util/mac/xattr.cc
diff --git a/util/mac/xattr.cc b/util/mac/xattr.cc
new file mode 100644
index 0000000000000000000000000000000000000000..dbbd1f120ade0d501e6dd34e32ec888d74ba344f
--- /dev/null
+++ b/util/mac/xattr.cc
@@ -0,0 +1,128 @@
+// 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/mac/xattr.h"
+
+#include <sys/xattr.h>
+
+#include "base/basictypes.h"
+#include "base/logging.h"
+#include "base/numerics/safe_conversions.h"
+#include "base/strings/stringprintf.h"
+#include "base/strings/string_number_conversions.h"
+
+namespace crashpad {
+
+bool ReadXattr(const base::FilePath& file,
+               const base::StringPiece& name,
+               std::string* value) {
+  value->resize(128);
+  ssize_t rv = 0;
+  do {
+    // getxattr() has a |position| argument, but it is only used for resource
+    // fork attributes. If reading the attribute filled up to |value|'s
+    // capacity, resize the buffer and try to read the entire attribute again.
+    rv = getxattr(file.value().c_str(), name.data(), &(*value)[0],

[Mark Mentovai, 2014/12/19 23:16:32]

I don’t like the new way clang-format wants us to write parameters, but I won’t
force you to change it if you like it. Same on line 58.

[Robert Sesek, 2014/12/30 17:02:21]

What's wrong with this? (I didn't clang-format but handwrote it).

[Mark Mentovai, 2014/12/30 19:19:15]

Three parameters on one line, three on the other, no real alignment to show that
they’re all parameters of equal importance, no real thought put into grouping
similar parameters…

Assuming everything doesn’t all fit on a single line, I usually do:

  SomeFunction(
      some_argument, some_other_argument, some_more_argumentative_stuff);

  AnotherFunction(allays,
                  eye,
                  bee,
                  eff);

We recently did https://codereview.chromium.org/805333006/ after discussing at
https://codereview.chromium.org/821483002/diff/20001/util/file/file_io_posix.....

+        value->capacity(), 0, 0);

[Mark Mentovai, 2014/12/19 23:16:32]

You need to use value->size() and not value->capacity(). If the capacity is
larger than the size, then you have enough space to read the xattr, but when you
subsequently resize() it, the portion beyond size() will be zeroed out.

[Robert Sesek, 2014/12/30 17:02:20]

Done.

+    if (rv < 0) {
+      if (errno == ERANGE) {

[Mark Mentovai, 2014/12/19 23:16:32]

#include <errno.h>

[Robert Sesek, 2014/12/30 17:02:20]

Done.

+        value->resize(value->capacity() * 2);

[Mark Mentovai, 2014/12/19 23:16:32]

Where’s the upper-bound protection?

[Robert Sesek, 2014/12/30 17:02:21]

I initially wrote upper-bound protection but later removed it. What would a
sensible value be?

+        continue;

[Mark Mentovai, 2014/12/19 23:16:32]

Get rid of this continue and put the PLOG/return that follows into an else.

The condition of the loop at the bottom is already testing rv < 0 && errno ==
ERANGE.

Or, leave this alone right here and change the condition down below to false,
relying only on the continue to cause the loop to run for another iteration.

[Robert Sesek, 2014/12/30 17:02:20]

Done.

+      }
+
+      PLOG(ERROR) << "ReadXattr " << name << " on file " << file.value();

[Mark Mentovai, 2014/12/19 23:16:32]

PLOGs normally show the name of the syscall, not the name of their own function,
so this should be getxattr instead of ReadXattr. Similar on line 60.

[Robert Sesek, 2014/12/30 17:02:21]

Done.

+      return false;
+    }
+
+    // Resize the buffer to be the length of the read string.
+    value->resize(rv);
+  } while (rv < 0 && errno == ERANGE);
+
+  return true;
+}
+
+bool WriteXattr(const base::FilePath& file,
+                const base::StringPiece& name,
+                const std::string& value) {
+  int rv = setxattr(file.value().c_str(), name.data(), value.c_str(),
+      value.length(), 0, 0);
+  PLOG_IF(ERROR, rv != 0) << "WriteXattr " << name << " on file "
+                          << file.value();
+  return rv == 0;
+}
+
+bool ReadXattrBool(const base::FilePath& file,
+                   const base::StringPiece& name,
+                   bool* value) {
+  std::string tmp;
+  if (!ReadXattr(file, name, &tmp))
+    return false;
+  if (tmp == "1") {
+    *value = true;
+    return true;
+  } else if (tmp == "0") {
+    *value = false;
+    return true;
+  } else {
+    return false;

[Mark Mentovai, 2014/12/19 23:16:31]

You need to log a message here to maintain the documented API contract.

[Robert Sesek, 2014/12/30 17:02:21]

Done.

+  }
+}
+
+bool WriteXattrBool(const base::FilePath& file,
+                    const base::StringPiece& name,
+                    bool value) {
+  return WriteXattr(file, name, (value ? "1" : "0"));

[Mark Mentovai, 2014/12/19 23:16:32]

The inner () aren’t necessary.

[Robert Sesek, 2014/12/30 17:02:20]

Indeed, but I think they help readability here.

+}
+
+bool ReadXattrInt(const base::FilePath& file,
+                  const base::StringPiece& name,
+                  int* value) {
+  std::string tmp;
+  if (!ReadXattr(file, name, &tmp))
+    return false;
+  return base::StringToInt(tmp, value);
+}
+
+bool WriteXattrInt(const base::FilePath& file,
+                   const base::StringPiece& name,
+                   int value) {
+  std::string tmp = base::StringPrintf("%d", value);
+  return WriteXattr(file, name, tmp);
+}
+
+bool ReadXattrTimeT(const base::FilePath& file,
+                    const base::StringPiece& name,
+                    time_t* value) {
+  // time_t on OS X is defined as a long, but it will be read into an
+  // int64_t here, since there is no string conversion method for long.
+  std::string tmp;
+  if (!ReadXattr(file, name, &tmp))
+    return false;
+
+  int64_t encoded_value;
+  if (!base::StringToInt64(tmp, &encoded_value))
+    return false;
+
+  *value = base::saturated_cast<time_t, int64_t>(encoded_value);

[Mark Mentovai, 2014/12/19 23:16:32]

I think this should log a warning if it saturates.

[Robert Sesek, 2014/12/30 17:02:21]

Done.

+  return true;
+}
+
+bool WriteXattrTimeT(const base::FilePath& file,
+                     const base::StringPiece& name,
+                     time_t value) {
+  std::string tmp = base::StringPrintf("%ld", value);
+  return WriteXattr(file, name, tmp);
+}
+
+}  // namespace crashpad