From 203bfaec37cdcf72dc9c9633866179d64795780f Mon Sep 17 00:00:00 2001 From: Iain Patterson Date: Thu, 25 Aug 2016 11:41:09 +0100 Subject: [PATCH] Seek to end of large files correctly. The documentation for SetFilePointer() is potentially confusing. When the third argument is NULL, the second argument is treated as a 32-bit offset to move the pointer. When the third argument is not NULL, it is a pointer to an unsigned integer representing the high order part of a 64-bit signed offset, and the second argument is treated as the corresponding (signed) low order part. The documentation states that "if you do not need the high order 32-bits" the third argument must be NULL, and indeed we were setting it to NULL on the basis that we were moving 0 bytes from FILE_END and 0 can most definitely be represented in 32-bits. However in practice when seeking to the end of a file whose size cannot be represented in 32-bits we actually need to use a 64-bit offset, even though the 64-bit integer we use is 0. So we must pass a pointer to a zeroed high part. Note that when SetFilePointer() succeeds the return value is the low order part of the actual offset and the high order value is filled with the high order offset, which has two important implications. Firstly we cannot simply declare our zero variable statically but must pass a new value each time. Secondly it is insufficient to check that the function returned something other than INVALID_SET_FILE_POINTER, as we were doing; instead we must treat that return value as success if and only iff GetLastError() returns NO_ERROR. Marcin Lewandowski noticed that large files were not always appended correctly. Because we were passing NULL as the third argument to SetFilePointer() it returned INVALID_SET_FILE_POINTER and we didn't call SetEndOfFile(). SetFilePointerEx() is supported on all versions of Windows we target, and has a much simpler definition than SetFilePointer(). We now use that instead. --- README.txt | 1 + io.cpp | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/README.txt b/README.txt index 35df5b4..00440f9 100644 --- a/README.txt +++ b/README.txt @@ -1005,6 +1005,7 @@ Thanks to David Bremner for general tidyups. Thanks to Nabil Redmann for suggesting redirecting hooks' output. Thanks to Bader Aldurai for suggesting the process tree. Thanks to Christian Long for suggesting virtual accounts. +Thanks to Marcin Lewandowski for spotting a bug appending to large files. Licence ------- diff --git a/io.cpp b/io.cpp index b6f9d37..ce455a3 100644 --- a/io.cpp +++ b/io.cpp @@ -174,9 +174,10 @@ int delete_createfile_parameter(HKEY key, TCHAR *prefix, TCHAR *suffix) { } HANDLE write_to_file(TCHAR *path, unsigned long sharing, SECURITY_ATTRIBUTES *attributes, unsigned long disposition, unsigned long flags) { + static LARGE_INTEGER offset = { 0 }; HANDLE ret = CreateFile(path, FILE_WRITE_DATA, sharing, attributes, disposition, flags, 0); - if (ret!= INVALID_HANDLE_VALUE) { - if (SetFilePointer(ret, 0, 0, FILE_END) != INVALID_SET_FILE_POINTER) SetEndOfFile(ret); + if (ret != INVALID_HANDLE_VALUE) { + if (SetFilePointerEx(ret, offset, 0, FILE_END)) SetEndOfFile(ret); return ret; } -- 2.20.1