fs: fully initialize `_OVERLAPPED` for win32 #26090

fs.cpp: In member function ‘bool fsbridge::FileLock::TryLock()’:
fs.cpp:129:32: error: missing initializer for member ‘_OVERLAPPED::InternalHigh’ [-Werror=missing-field-initializers]
  129 |     _OVERLAPPED overlapped = {0};
      |                                ^
fs.cpp:129:32: error: missing initializer for member ‘_OVERLAPPED::<anonymous>’ [-Werror=missing-field-initializers]
fs.cpp:129:32: error: missing initializer for member ‘_OVERLAPPED::hEvent’ [-Werror=missing-field-initializers]

Came up in #25972. That PR is now rebased on this change.

Closes: #26006

This is a refactor?

ACK 02c9e564687af6ae2b0b6589108d502963f879cb, tested on Linux x86_64:

• on the master branch (13fd9ee5c2d747e9f74d3fd8e33a4f0c9eaa769c):

$ make -j $(nproc) -C depends HOST=x86_64-w64-mingw32
$ ./autogen.sh
$ ./configure -q CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site CXXFLAGS="-Wno-return-type -Wmissing-field-initializers"
$ make clean
$ make -j $(nproc) > /dev/null 
fs.cpp: In member function ‘bool fsbridge::FileLock::TryLock()’:
fs.cpp:129:32: warning: missing initializer for member ‘_OVERLAPPED::InternalHigh’ [-Wmissing-field-initializers]
  129 |     _OVERLAPPED overlapped = {0};
      |                                ^
fs.cpp:129:32: warning: missing initializer for member ‘_OVERLAPPED::<anonymous>’ [-Wmissing-field-initializers]
fs.cpp:129:32: warning: missing initializer for member ‘_OVERLAPPED::hEvent’ [-Wmissing-field-initializers]
leveldb/util/env_windows.cc: In member function ‘virtual leveldb::Status leveldb::{anonymous}::WindowsRandomAccessFile::Read(uint64_t, size_t, leveldb::Slice*, char*) const’:
leveldb/util/env_windows.cc:197:31: warning: missing initializer for member ‘_OVERLAPPED::InternalHigh’ [-Wmissing-field-initializers]
  197 |     OVERLAPPED overlapped = {0};
      |                               ^
leveldb/util/env_windows.cc:197:31: warning: missing initializer for member ‘_OVERLAPPED::<anonymous>’ [-Wmissing-field-initializers]
leveldb/util/env_windows.cc:197:31: warning: missing initializer for member ‘_OVERLAPPED::hEvent’ [-Wmissing-field-initializers]

• with this PR (02c9e564687af6ae2b0b6589108d502963f879cb):

$ make -j $(nproc) -C depends HOST=x86_64-w64-mingw32
$ ./autogen.sh
$ ./configure -q CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site CXXFLAGS="-Wno-return-type -Wmissing-field-initializers"
$ make clean
$ make -j $(nproc) > /dev/null 
leveldb/util/env_windows.cc: In member function ‘virtual leveldb::Status leveldb::{anonymous}::WindowsRandomAccessFile::Read(uint64_t, size_t, leveldb::Slice*, char*) const’:
leveldb/util/env_windows.cc:197:31: warning: missing initializer for member ‘_OVERLAPPED::InternalHigh’ [-Wmissing-field-initializers]
  197 |     OVERLAPPED overlapped = {0};
      |                               ^
leveldb/util/env_windows.cc:197:31: warning: missing initializer for member ‘_OVERLAPPED::<anonymous>’ [-Wmissing-field-initializers]
leveldb/util/env_windows.cc:197:31: warning: missing initializer for member ‘_OVERLAPPED::hEvent’ [-Wmissing-field-initializers]

Maybe submit a similar change to leveldb upstream as well?

tACK 02c9e564687af6ae2b0b6589108d502963f879cb.

Maybe submit a similar change to leveldb upstream as well?

I've submitted the same change upstream, https://github.com/google/leveldb/pull/1053, however given the "very limited maintenance" state of leveldb, I think the likelyhood of it being merged is low. We could pull a similar change into our subtree if we wanted.

Also opened a PR with the same change as sent upstream, in our subtree repo: https://github.com/bitcoin-core/leveldb-subtree/pull/34.

Post-merge comment:

From: https://learn.microsoft.com/en-us/windows/win32/api/minwinbase/ns-minwinbase-overlapped

Any unused members of this structure should always be initialized to zero before the structure is used in a function call. Otherwise, the function may fail and return ERROR_INVALID_PARAMETER.

According to that note at least, it's possible that this was always failing before.

This commit is obviously correct and a fix, we should still investigate whether or not this changes behavior in any meaningful way. Though I suspect we would've noticed, as LockDataDirectory in init.cpp effectively acts as a sanity check for this.