Closed Bug 945148 Opened 6 years ago Closed 6 years ago

when building toolkit/library/nsDllMain.cpp: "delayimp.h(107) : warning C4005: 'FACILITY_VISUALCPP' : macro redefinition"

Categories

(Core :: General, defect)

All
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28
Tracking Status
firefox28 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qa-])

Attachments

(1 file)

Build warning:
{
c:\PROGRA~2\MICROS~2.0\vc\include\delayimp.h(107) : warning C4005: 'FACILITY_VISUALCPP' : macro redefinition
        c:\Program Files (x86)\Windows Kits\8.0\include\shared\winerror.h(95) : see previous definition of 'FACILITY_VISUALCPP'
}
https://tbpl.mozilla.org/php/getParsedLog.php?id=31298282&tree=Try

On my local system, delayimp.h actually has this:
> //#define FACILITY_VISUALCPP  ((LONG)0x6d)  now defined in winerror.h
(note that the #define is commented out now.)

(This is /c/Program Files (x86)/Microsoft Visual Studio 11.0/VC/include/delayimp.h )

Presumably the MSVC version on our tinderboxen is a bit older and has that #define uncommented, which is causing the problem.
(I call it "the problem" because it appears this is the only build warning in toolkit/library, as shown by this Try run: https://tbpl.mozilla.org/?tree=Try&rev=28f7e07a30d7
With this fixed, we can label toolkit/library as warning-free.)
Some googling turned up
 http://git.chromium.org/gitweb/?p=chromium.git;a=commitdiff;h=18ba6c50c7595861a5140eb1ce21a77bc71d0314
which seems to have a SDK-version-based hackaround for this issue.
Actually, this file has a more complete fix, based both on the SDK version and MSVC version:
 https://chromium.googlesource.com/chromium/chromium/+/trunk/chrome/app/delay_load_hook_unittest_win.cc

Quoting:
> #include <windows.h>
> #if defined(_WIN32_WINNT_WIN8) && _MSC_VER < 1700
> // The Windows 8 SDK defines FACILITY_VISUALCPP in winerror.h, and in
> // delayimp.h previous to VS2012.
> #undef FACILITY_VISUALCPP
> #endif
> #include <DelayIMP.h>

We probably want something like that.
Attached patch fix v1Splinter Review
This is similar to the conditional #undef quoted above, but with a slightly improved condition (checking for _MSC_VER being defined before comparing it), and with a finessed comment.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #8340952 - Flags: review?(benjamin)
Blocks: 945151
Attachment #8340952 - Flags: review?(benjamin) → review+
https://hg.mozilla.org/mozilla-central/rev/3930e0fc40ac
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.