Refactor AppendToFile and WriteFileDescriptor

base/files/file_util.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This file contains utility functions for dealing with the local
 // filesystem.
 
 #ifndef BASE_FILES_FILE_UTIL_H_
 #define BASE_FILES_FILE_UTIL_H_
 
@@ skipping 308 matching lines @@
 // Reads at most the given number of bytes from the file into the buffer.
 // Returns the number of read bytes, or -1 on error.
 BASE_EXPORT int ReadFile(const FilePath& filename, char* data, int max_size);
 
 // Writes the given buffer into the file, overwriting any data that was
 // previously there.  Returns the number of bytes written, or -1 on error.
 BASE_EXPORT int WriteFile(const FilePath& filename, const char* data,
                           int size);
 
 #if defined(OS_POSIX)
-// Append the data to |fd|. Does not close |fd| when done.
-BASE_EXPORT int WriteFileDescriptor(const int fd, const char* data, int size);
+// Append |data| to |fd|. Does not close |fd| when done.  Returns true if all of

[rvargas (doing something else), 2014/10/01 20:01:14]

nit: appends

+// |data| was written, false otherwise.  In case of error, guarantees that the
+// last system error is preserved.
+BASE_EXPORT bool WriteFileDescriptor(const int fd, const char* data, int size);
 #endif
 
-// Append the given buffer into the file. Returns the number of bytes written,
-// or -1 on error.
-BASE_EXPORT int AppendToFile(const FilePath& filename,
-                             const char* data, int size);
+// Append |data| to |filename|.  Returns true if all of |data| was written,
+// false otherwise.  In case of error, guarantees that the last system error is

[rvargas (doing something else), 2014/10/01 20:01:14]

I'm a on the fence about the last error bit. There is only one place on all the
file related code from base where we explicitly preserve last error (and it is
actually not a documented feature). It seems like all callers are happy not
knowing the exact reason for the failure, as the current code doesn't preserve
it.

It is quite easy to preserve the error... just a few extra lines, but then we
start to lose uniformity as almost nowhere else the error is preserved (it may,
but it may not). On the other hand, it makes the code more flexible.

Brett, any opinion?

+// preserved.
+BASE_EXPORT bool AppendToFile(const FilePath& filename,
+                              const char* data,
+                              int size);
 
 // Gets the current working directory for the process.
 BASE_EXPORT bool GetCurrentDirectory(FilePath* path);
 
 // Sets the current working directory for the process.
 BASE_EXPORT bool SetCurrentDirectory(const FilePath& path);
 
 // Attempts to find a number that can be appended to the |path| to make it
 // unique. If |path| does not exist, 0 is returned.  If it fails to find such
 // a number, -1 is returned. If |suffix| is not empty, also checks the
@@ skipping 83 matching lines @@
 // This function simulates Move(), but unlike Move() it works across volumes.
 // This function is not transactional.
 BASE_EXPORT bool CopyAndDeleteDirectory(const FilePath& from_path,
                                         const FilePath& to_path);
 #endif  // defined(OS_WIN)
 
 }  // namespace internal
 }  // namespace base
 
 #endif  // BASE_FILES_FILE_UTIL_H_