Make base:ReplaceFile return an informative error.

base/file_util.h

Index: base/file_util.h
diff --git a/base/file_util.h b/base/file_util.h
index 4ffd05babda8018f625cbf237d2101a058604bae..052bb85e3cb15611da1e5ae279c4b5c1a3529e98 100644
--- a/base/file_util.h
+++ b/base/file_util.h
@@ -101,9 +101,11 @@ BASE_EXPORT bool MoveUnsafe(const base::FilePath& from_path,
 // volume, or the function will fail. Destination file will be created
 // if it doesn't exist. Prefer this function over Move when dealing with
 // temporary files. On Windows it preserves attributes of the target file.
-// Returns true on success.
+// Returns true on success and optionally sets *error to base::PLATFORM_FILE_OK.

[jar (doing other things), 2013/05/07 01:12:36]

nit: I think you should avoid specing things as "optional."
Better (and simpler?) would be return true on success, and does not change the
value of |error|.  Returns false if there is an error, and places an the values
into |*error| if |error| is non-null.

[dgrogan, 2013/05/07 19:02:05]

That sounds fine. But it will require changing all the callers. I was aiming for
least disruptive. But if you think doing that is the way to go then I'm all for
it.

[jar (doing other things), 2013/05/07 19:44:50]

Default args would indeed be less disruptive... but that is very clearly against
the style guide.

The suggestion about removing "optionally sets" and specing it to always set the
errorr IFF returning false shouldn't make it more disruptive.

It is fine to have the "no arg" method call the method with a return |*error|
arg, so that your code doesn't have to be replicated.

+// Returns false on failure and optionally sets *error appropriately.
 BASE_EXPORT bool ReplaceFile(const base::FilePath& from_path,
-                             const base::FilePath& to_path);
+                             const base::FilePath& to_path,
+                             base::PlatformFileError* error = NULL);

[jar (doing other things), 2013/05/07 01:12:36]

nit: default arguments are not allowed according to the style guide.  This sure
seems like a plausibly reasonable exception... but there are none of this sort.

 
 // Copies a single file. Use CopyDirectory to copy directories.
 // This function fails if either path contains traversal components ('..').