Add LoggingOpenFileFor{Read|Write}

util/file/file_io.h

 // 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.
 
 #ifndef CRASHPAD_UTIL_FILE_FILE_IO_H_
 #define CRASHPAD_UTIL_FILE_FILE_IO_H_
 
 #include <sys/types.h>
 
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
 #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.
 //!
 //! This function reads into \a buffer, stopping only when \a size bytes have
 //! been read or when end-of-file has been reached. On Windows, reading from
 //! sockets is not currently supported.
 //!
 //! \return The number of bytes read and placed into \a buffer, or `-1` on
 //!     error, with `errno` or `GetLastError()` set appropriately. On error, a
 //!     portion of \a file may have been read into \a buffer.
@@ skipping 70 matching lines @@
 //! \brief Wraps ReadFile(), ensuring that it indicates end-of-file.
 //!
 //! Attempts to read a single byte from \a file, expecting no data to be read.
 //! If the underlying ReadFile() fails, or if a byte actually is read, this
 //! function causes execution to terminate without returning.
 //!
 //! \sa CheckedReadFile
 //! \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.
 //!
 //! \return On success, `true` is returned. On failure, an error is logged and
 //!     `false` is returned.
 bool LoggingCloseFile(FileHandle file);
 
 //! \brief Wraps `close()` or `CloseHandle()`, ensuring that it succeeds.
 //!
 //! If the underlying function fails, this function causes execution to
 //! terminate without returning.
 void CheckedCloseFile(FileHandle file);
 
 }  // namespace crashpad
 
 #endif  // CRASHPAD_UTIL_FILE_FILE_IO_H_