Add LoggingOpenFileFor{Read|Write}

util/file/file_io.h

Index: util/file/file_io.h
diff --git a/util/file/file_io.h b/util/file/file_io.h
index 927bc3cb13319ff5843a613cff77bbfd20588648..571d5d0024f14ba8f8bd1bad3eaf9e7f7b9ae93e 100644
--- a/util/file/file_io.h
+++ b/util/file/file_io.h
@@ -23,14 +23,37 @@
 #include <windows.h>
 #endif
 
+namespace base {
+class FilePath;
+}  // namespace base
+
 namespace crashpad {
 
-#if defined(OS_POSIX)
+#if defined(OS_POSIX) || DOXYGEN
+//! \brief Platform-specific alias for a low level file handle.
 using FileHandle = int;
 #elif defined(OS_WIN)
 using FileHandle = HANDLE;
 #endif
 
+//! \brief Determines the mode that LoggingOpenFileForWrite uses.

[Mark Mentovai, 2014/12/18 20:24:59]

() on function names.

[scottmg, 2014/12/18 21:18:54]

Done.

+enum FileWriteMode {
+  //! \brief Opens the file if it exists and seeks to the end of the file, or

[Mark Mentovai, 2014/12/18 20:24:59]

Meh, I don’t know about having this actually do the seek.

[scottmg, 2014/12/18 21:18:53]

OK, removed that part. (Next CL will probably be LoggingSeekFile, now that I
think about it...)

+  //!        creates a new file.
+  kAppend,
+
+  //! \brief Creates a new file. If the file already exists, it will be
+  //!        overwritten.

[Mark Mentovai, 2014/12/18 20:24:59]

Overwritten? What’s the difference between kCreate and kTruncate? I don’t think
we can maintain that distinction on POSIX.

[scottmg, 2014/12/18 21:18:54]

I meant for kTruncate to be open-but-must-already-exist. I was vaguely thinking
of some sort of temp file being created elsewhere and having the name passed to
here. But as we don't have a need, I'll remove it.

[Mark Mentovai, 2014/12/18 21:52:32]

scottmg wrote:
Hopefully if we have a need for that, we’ll be able to pass the FileHandle
instead of or alongside the path.

+  kCreate,
+
+  //! \brief Creates a new file. If the file already exists, the open will fail.
+  kCreateNew,
+
+  //! \brief Opens an existing file. When the file is opened, it will be
+  //!        truncated so that its size is zero bytes.
+  kTruncate,

[Mark Mentovai, 2014/12/18 20:27:34]

Oh, also, these names should be something like kFileWriteModeTruncate. Since
they’re just in namespace crashpad and not in a class or anything, they should
be a little more descriptive, because they’ll be used throughout the namespace
just by their bare names.

Or maybe this is a good application for this codebase’s very first “enum class”!

[scottmg, 2014/12/18 21:18:53]

I'll go with enum class then for the excitement and glory!

+};

[Mark Mentovai, 2014/12/18 20:24:59]

Everything here has the potential to create a file. The three options I see are:

 - require that the file be created
 - allow the file to exist but discard existing contents (whether it’s via
truncation or replacing it
 - allow the file to exist and retain existing contents

[scottmg, 2014/12/18 21:18:53]

That sounds right after removing kTruncate. So the first is kCreateNew, the
second is kCreate, and the third kAppend. Better name suggestions?

+
 //! \brief Reads from a file, retrying when interrupted on POSIX or following a
 //!        short read.
 //!
@@ -121,6 +144,32 @@ void CheckedWriteFile(FileHandle file, const void* buffer, size_t size);
 //! \sa ReadFile
 void CheckedReadFileAtEOF(FileHandle file);
 
+//! \brief Wraps `open()` or `CreateFile()`, creating a file for output. Logs
+//!        an error if the operation fails.
+//!
+//! \a write_mode determines the style (clobber, truncate, etc.) that is used

[Mark Mentovai, 2014/12/18 20:24:59]

Normally parameters are done with \param[in] style but the other functions in
here don’t have that, so this is OK.

[scottmg, 2014/12/18 21:18:54]

OK. LMK if you want to change them all and I can do it in a separate CL.
(Unrelatedly, are the "!" necessary in the comment blocks? Vim (or my config of
Vim?) doesn't deal well with them which make them tedious to edit.)

[scottmg, 2014/12/18 21:36:45]

Answering my own parenthetical,
http://www.stack.nl/~dimitri/doxygen/manual/docblocks.html yes, either //! or
///. Vim doesn't do seem to do well with either, oh well.

[Mark Mentovai, 2014/12/18 21:52:32]

scottmg wrote:
No rush, but it’d be good to be consistent with the rest of the documentation.

I think we wound up in this state because sometimes, when I have functions that
just wrap others, there’s not much point in documenting the parameters
individually, so I just leave it out. But some of these, at least, are no longer
straight wrappers.
There are a few different comment styles for Doxygen. See
http://www.doxygen.org/manual/docblocks.html. Feel free to experiment with the
other alternatives, maybe one of them works better with editors, and we can do a
mass scripted rewrite. But in my experience, the other alternatives typically
work as poorly as //!, and the advantage of //! over /// is that it’s more
immediately visible as distinct from //.

It’s possible to change many editors’ ideas of what a comment looks like, and
sometimes it’s possible to tell an editor that //! is distinct from // and takes
precedence.

+//! to open the file. On POSIX, if \a world_readable, the permissions bits will
+//! be set to 0644, otherwise, 0600. On Windows, the file is always opened in

[Mark Mentovai, 2014/12/18 20:24:59]

Numeric literals also get the `backtick` treatment.

[Mark Mentovai, 2014/12/18 20:24:59]

“set to” is not totally correct, because the umask will still modify the bits.

[scottmg, 2014/12/18 21:18:53]

Done.

[scottmg, 2014/12/18 21:18:53]

Attempted improvement.

+//! binary mode (that is, no CRLF translation).
+//!
+//! \return The newly opened FileHandle, or an invalid FileHandle on failure.
+//!
+//! \sa FileWriteMode
+//! \sa ScopedFD
+//! \sa ScopedFileHANDLE
+FileHandle LoggingOpenFileForWrite(const base::FilePath& path,
+                                   FileWriteMode write_mode,
+                                   bool world_readable);
+
+//! \brief Wraps `open()` or `CreateFile()`, opening an existing file for
+//!        reading. Logs an error if the operation fails.
+//!
+//! \return The newly opened FileHandle, or an invalid FileHandle on failure.
+//!
+//! \sa ScopedFD
+//! \sa ScopedFileHANDLE
+FileHandle LoggingOpenFileForRead(const base::FilePath& path);
+
 //! \brief Wraps `close()` or `CloseHandle()`, logging an error if the operation
 //!        fails.
 //!