Skip to content

Commit

Permalink
Define WIN32_LEAN_AND_MEAN conditionally.
Browse files Browse the repository at this point in the history 
Fixes: #823
PR-URL: #824
Reviewed-By: Bert Belder <bertbelder@gmail.com>
  • Loading branch information
@bnoordhuis
bnoordhuis committed Nov 25, 2015
1 parent 328d671 commit 90078ec
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/win_delay_load_hook.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@

#ifdef _MSC_VER

#ifndef WIN32_LEAN_AND_MEAN
#define WIN32_LEAN_AND_MEAN
#endif

#include <windows.h>

#include <delayimp.h>
Expand Down

3 comments on commit 90078ec

@eljefedelrodeodeljefe
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A question unrelated to that commit, but rather the whole file: Can this mechanism theoretically be ignored on newer versions of node? This would probably just affect some users prior to 4.X right? Or is it integral to addon compilation?

@Flarna
Copy link
Member

@Flarna Flarna commented on 90078ec Jun 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eljefedelrodeodeljefe: To my understanding it's needed with any version of node to support to use renamed node.exe and/or iojs.exe. As long as you (and the users of your native addon) keep using only node.exe it's not needed to use this mechanism (not even for older node versions).

In one of my projects (where I can't use node gyp for some unrelated reason) I skipped adding win_delay_load_hook.c in my build setup. Reason was that adding the linker switch /DELAYLOAD caused linker errors during import of some vtables once I added more and more code using node/nan/v8 APIs.

Seems to be related to the delayload constraints documented at https://msdn.microsoft.com/en-us/library/yx1x886y.aspx

@eljefedelrodeodeljefe
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Flarna okay. That's what I thought and thats reassuring. I'll try going without it. And thanks for the link!!

Please sign in to comment.