On 2011/02/21 10:37:56, Mikhail Naganov (Chromium) wrote:
http://codereview.chromium.org/6543039/diff/1/src/platform-linux.cc
File src/platform-linux.cc (right):
http://codereview.chromium.org/6543039/diff/1/src/platform-linux.cc#newcode330
src/platform-linux.cc:330: FILE* file = fopen(name, "a+");
I don't think "a+" is the right flag. If you plan to read and write to the
file
from the beginning, and avoid truncating it, then you should use "r+".
This
concern has arisen to me because in 'mmap' you pass 0 as the offset
inside the
file, which contradicts with the "a" flag semantics that means appending
data
to
a file. Thus, using both "a" and "0" offset seems like a contradiction.
Also,
you don't pass 'FILE_APPEND_DATA' to the Win32 function, so I'm
concluding you
really don't want to append to the file.
Thanks for catching this.
At first, I was thinking that I would want the file to be created if it
does not
already exists. That's why I chose "a+".
http://www.cplusplus.com/reference/clibrary/cstdio/fopen/ indicates that it
will
simply fail if the file does not exist. But you are right about the append
write issue. On further consideration, "r+" is the right mode to use. I
doubled check that the error handling does cover the case of the file not
already existing.
Hence, I reverted the win32 case to OPEN_EXISTING, and corrected the return
value check to check for INVALID_HANDLE_VALUE instead of NULL (according to
msdn
specs).
http://codereview.chromium.org/6543039/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
