From: Iain Patterson <me@iain.cx>
Date: Thu, 25 Aug 2016 10:41:09 +0000 (+0100)
Subject: Seek to end of large files correctly.
X-Git-Url: http://git.iain.cx/?a=commitdiff_plain;h=203bfaec37cdcf72dc9c9633866179d64795780f;p=nssm.git

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.
---

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;
   }