> -----Original Message-----
> From: Johannes Schindelin [mailto:johannes.schinde...@gmx.de]
> Sent: Monday, September 18, 2017 10:25 AM
> To: Ben Peart <ben.pe...@microsoft.com>
> Cc: david.tur...@twosigma.com; ava...@gmail.com;
> christian.cou...@gmail.com; git@vger.kernel.org; gits...@pobox.com;
> pclo...@gmail.com; p...@peff.net
> Subject: Re: [PATCH v6 12/12] fsmonitor: add a performance test
>
> Hi Ben,
>
> sorry for not catching this earlier:
>
> On Fri, 15 Sep 2017, Ben Peart wrote:
>
> > [...]
> > +
> > +int cmd_dropcaches(void)
> > +{
> > + HANDLE hProcess = GetCurrentProcess();
> > + HANDLE hToken;
> > + int status;
> > +
> > + if (!OpenProcessToken(hProcess, TOKEN_QUERY |
> TOKEN_ADJUST_PRIVILEGES, &hToken))
> > + return error("Can't open current process token");
> > +
> > + if (!GetPrivilege(hToken, "SeProfileSingleProcessPrivilege", 1))
> > + return error("Can't get SeProfileSingleProcessPrivilege");
> > +
> > + CloseHandle(hToken);
> > +
> > + HMODULE ntdll = LoadLibrary("ntdll.dll");
>
Thanks Johannes, I'll fix that.
> Git's source code still tries to abide by C90, and for simplicity's sake, this
> extends to the Windows-specific part. Therefore, the `ntdll` variable needs to
> be declared at the beginning of the function (I do agree that it makes for
> better code to reduce the scope of variables, but C90 simply did not allow
> variables to be declared in the middle of functions).
>
> I wanted to send a patch address this in the obvious way, but then I
> encountered these lines:
>
> > + DWORD(WINAPI *NtSetSystemInformation)(INT, PVOID, ULONG) =
> > + (DWORD(WINAPI *)(INT, PVOID,
> ULONG))GetProcAddress(ntdll, "NtSetSystemInformation");
> > + if (!NtSetSystemInformation)
> > + return error("Can't get function addresses, wrong Windows
> > +version?");
>
> It turns out that we have seen this plenty of times in Git for Windows'
> fork, so much so that we came up with a nice helper to make this all a bit
> more robust and a bit more obvious, too: the DECLARE_PROC_ADDR and
> INIT_PROC_ADDR helpers in compat/win32/lazyload.h.
>
> Maybe this would be the perfect excuse to integrate this patch into upstream
> Git?
This patch is pretty hefty already. How about you push this capability
upstream and I take advantage of it in a later patch. :)
This would be the patch (you can also cherry-pick it from
> 25c4dc3a73352e72e995594cf1b4afa46e93d040 in
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.
> com%2Fdscho%2Fgit&data=02%7C01%7CBen.Peart%40microsoft.com%7C96
> 4027bdc1f34a033c1f08d4fea1056e%7C72f988bf86f141af91ab2d7cd011db47
> %7C1%7C0%7C636413414914282865&sdata=jyvu6G7myRY9UA1XxWx2tDZ%
> 2BWsIWqLTRMT8WfzEGe5g%3D&reserved=0):
>
> -- snip --
> From 25c4dc3a73352e72e995594cf1b4afa46e93d040 Mon Sep 17 00:00:00
> 2001
> From: Johannes Schindelin <johannes.schinde...@gmx.de>
> Date: Tue, 10 Jan 2017 23:14:20 +0100
> Subject: [PATCH] Win32: simplify loading of DLL functions
>
> Dynamic loading of DLL functions is duplicated in several places in Git for
> Windows' source code.
>
> This patch adds a pair of macros to simplify the process: the
> DECLARE_PROC_ADDR(<dll>, <return-type>, <function-name>,
> ...<function-parameter-types>...) macro to be used at the beginning of a
> code block, and the INIT_PROC_ADDR(<function-name>) macro to call before
> using the declared function. The return value of the INIT_PROC_ADDR() call
> has to be checked; If it is NULL, the function was not found in the specified
> DLL.
>
> Example:
>
> DECLARE_PROC_ADDR(kernel32.dll, BOOL, CreateHardLinkW,
> LPCWSTR, LPCWSTR, LPSECURITY_ATTRIBUTES);
>
> if (!INIT_PROC_ADDR(CreateHardLinkW))
> return error("Could not find CreateHardLinkW() function";
>
> if (!CreateHardLinkW(source, target, NULL))
> return error("could not create hardlink from %S to %S",
> source, target);
> return 0;
>
> Signed-off-by: Karsten Blees <bl...@dcon.de>
> Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de>
> ---
> compat/win32/lazyload.h | 44
> ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
> create mode 100644 compat/win32/lazyload.h
>
> diff --git a/compat/win32/lazyload.h b/compat/win32/lazyload.h new file
> mode 100644 index 00000000000..91c10dad2fb
> --- /dev/null
> +++ b/compat/win32/lazyload.h
> @@ -0,0 +1,44 @@
> +#ifndef LAZYLOAD_H
> +#define LAZYLOAD_H
> +
> +/* simplify loading of DLL functions */
> +
> +struct proc_addr {
> + const char *const dll;
> + const char *const function;
> + FARPROC pfunction;
> + unsigned initialized : 1;
> +};
> +
> +/* Declares a function to be loaded dynamically from a DLL. */ #define
> +DECLARE_PROC_ADDR(dll, rettype, function, ...) \
> + static struct proc_addr proc_addr_##function = \
> + { #dll, #function, NULL, 0 }; \
> + static rettype (WINAPI *function)(__VA_ARGS__)
> +
> +/*
> + * Loads a function from a DLL (once-only).
> + * Returns non-NULL function pointer on success.
> + * Returns NULL + errno == ENOSYS on failure.
> + */
> +#define INIT_PROC_ADDR(function) \
> + (function = get_proc_addr(&proc_addr_##function))
> +
> +static inline void *get_proc_addr(struct proc_addr *proc) {
> + /* only do this once */
> + if (!proc->initialized) {
> + HANDLE hnd;
> + proc->initialized = 1;
> + hnd = LoadLibraryExA(proc->dll, NULL,
> + LOAD_LIBRARY_SEARCH_SYSTEM32);
> + if (hnd)
> + proc->pfunction = GetProcAddress(hnd, proc-
> >function);
> + }
> + /* set ENOSYS if DLL or function was not found */
> + if (!proc->pfunction)
> + errno = ENOSYS;
> + return proc->pfunction;
> +}
> +
> +#endif
> -- snap --
>
> With this patch, this fixup to your patch would make things compile (you can
> also cherry-pick d05996fb61027512b8ab31a36c4a7a677dea11bb from my
> fork):
>
> -- snipsnap --
> From d05996fb61027512b8ab31a36c4a7a677dea11bb Mon Sep 17 00:00:00
> 2001
> From: Johannes Schindelin <johannes.schinde...@gmx.de>
> Date: Mon, 18 Sep 2017 14:56:40 +0200
> Subject: [PATCH] fixup! fsmonitor: add a performance test
>
> ---
> t/helper/test-drop-caches.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/t/helper/test-drop-caches.c b/t/helper/test-drop-caches.c index
> 717079865cb..b27358528f7 100644
> --- a/t/helper/test-drop-caches.c
> +++ b/t/helper/test-drop-caches.c
> @@ -1,6 +1,7 @@
> #include "git-compat-util.h"
>
> #if defined(GIT_WINDOWS_NATIVE)
> +#include "compat/win32/lazyload.h"
>
> int cmd_sync(void)
> {
> @@ -82,6 +83,9 @@ int cmd_dropcaches(void)
> HANDLE hProcess = GetCurrentProcess();
> HANDLE hToken;
> int status;
> + SYSTEM_MEMORY_LIST_COMMAND command;
> + DECLARE_PROC_ADDR(ntll,
> + DWORD, NtSetSystemInformation, INT, PVOID,
> ULONG);
>
> if (!OpenProcessToken(hProcess, TOKEN_QUERY |
> TOKEN_ADJUST_PRIVILEGES, &hToken))
> return error("Can't open current process token"); @@ -91,16
> +95,10 @@ int cmd_dropcaches(void)
>
> CloseHandle(hToken);
>
> - HMODULE ntdll = LoadLibrary("ntdll.dll");
> - if (!ntdll)
> - return error("Can't load ntdll.dll, wrong Windows
> version?");
> -
> - DWORD(WINAPI *NtSetSystemInformation)(INT, PVOID, ULONG) =
> - (DWORD(WINAPI *)(INT, PVOID,
> ULONG))GetProcAddress(ntdll,
> "NtSetSystemInformation");
> - if (!NtSetSystemInformation)
> + if (!INIT_PROC_ADDR(NtSetSystemInformation))
> return error("Can't get function addresses, wrong Windows
> version?");
>
> - SYSTEM_MEMORY_LIST_COMMAND command =
> MemoryPurgeStandbyList;
> + command = MemoryPurgeStandbyList;
> status = NtSetSystemInformation(
> SystemMemoryListInformation,
> &command,
> @@ -111,8 +109,6 @@ int cmd_dropcaches(void)
> else if (status != STATUS_SUCCESS)
> error("Unable to execute the memory list command %d",
> status);
>
> - FreeLibrary(ntdll);
> -
> return status;
> }
>
> --
> 2.14.1.windows.1.510.g0cb6d35d23
