Make sure we close the file_handle when we create (but not open) a new file

base/file_util_proxy.h

 // Copyright (c) 2010 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.
 
 #ifndef BASE_FILE_UTIL_PROXY_H_
 #define BASE_FILE_UTIL_PROXY_H_
 
 #include <vector>
 
 #include "base/callback.h"
@@ skipping 30 matching lines @@
   // Creates or opens a file with the given flags.  It is invalid to pass NULL
   // for the callback.
   typedef Callback3<base::PlatformFileError /* error code */,
                     base::PassPlatformFile,
                     bool /* created */>::Type CreateOrOpenCallback;
   static bool CreateOrOpen(scoped_refptr<MessageLoopProxy> message_loop_proxy,
                            const FilePath& file_path,
                            int file_flags,
                            CreateOrOpenCallback* callback);
 
+  // Creates a file with the given flags.  This one is a variation of
+  // CreateOrOpen but it doesn't return a file handle.

[brettw, 2010/10/13 22:48:32]

Sorry I didn't look at this before it was checked in.

From looking at the function name, Create is a bit confusing. I would normally
expect from an API with functions Create and CreateOrOpen that Create forces the
file to be created, giving me an empty file no matter what, while CreateOrOpen
would create me an empty one or open an existing one. I would expect both to
give me the handle.

I wonder if there's some name like EnsureFileExists that might make the behavior
more clear?

Can you also specify in the comment what the value will be in the callback?

[kinuko, 2010/10/13 23:01:28]

I see the point, sorry for bringing the confusing change.

EnsureFileExists sounds clearer, but we want it to return an error
(ERROR_FILE_EXIST, sort of EEXIST) if the file already exists and the method
name may not make sense in the case?

Hmm what if I simply remove this new method and add a flag like
'close_handle_before_callback' to CreateOrOpen?  (Or add an overloaded
CreateOrOpen that takes extra flags)
I will.

+  static bool Create(scoped_refptr<MessageLoopProxy> message_loop_proxy,
+                     const FilePath& file_path,
+                     int file_flags,
+                     CreateOrOpenCallback* callback);
+
   // Creates a temporary file for writing.  The path and an open file handle
   // are returned.  It is invalid to pass NULL for the callback.
   typedef Callback3<base::PlatformFileError /* error code */,
                     base::PassPlatformFile,
                     FilePath>::Type CreateTemporaryCallback;
   static bool CreateTemporary(
       scoped_refptr<MessageLoopProxy> message_loop_proxy,
       CreateTemporaryCallback* callback);
 
   // Close the given file handle.
@@ skipping 129 matching lines @@
       base::PlatformFile file,
       StatusCallback* callback);
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(FileUtilProxy);
 };
 
 }  // namespace base
 
 #endif  // BASE_FILE_UTIL_PROXY_H_