Avoid redefining ERROR in ErrorList.h when wingdi.h is also included

RESOLVED FIXED in mozilla19

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: jacek, Assigned: jacek)

Tracking

Trunk
mozilla19
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

fix
1.44 KB, patch
Ehsan
: review+
Details | Diff | Splinter Review
Assignee

Description

7 years ago
Posted patch fix v1.0 (obsolete) — Splinter Review
wingdi.h (a code Windows SDK header) defines ERROR to 0.
Attachment #671384 - Flags: review?(ehsan)
Assignee

Updated

7 years ago

Comment 1

7 years ago
Comment on attachment 671384 [details] [diff] [review]
fix v1.0

Review of attachment 671384 [details] [diff] [review]:
-----------------------------------------------------------------

Couldn't you just #undef ERROR if it had been defined previously?  That seems like a much simpler and more robust fix.

::: xpcom/base/nsError.h
@@ -12,5 @@
>  #include "mozilla/Attributes.h"
>  
> -#if defined(__cplusplus)
> -#include "mozilla/Attributes.h" // for MOZ_HAVE_CXX11_STRONG_ENUMS
> -#endif

Why are you removing this?
Attachment #671384 - Flags: review?(ehsan)
Assignee

Comment 2

7 years ago
(In reply to Ehsan Akhgari [:ehsan] from comment #1)
> Comment on attachment 671384 [details] [diff] [review]
> fix v1.0
> 
> Review of attachment 671384 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Couldn't you just #undef ERROR if it had been defined previously?  That
> seems like a much simpler and more robust fix.

Sure I could, but I consider avoiding collisions cleaner than hacking around them. I will prepare a new patch.

> ::: xpcom/base/nsError.h
> @@ -12,5 @@
> >  #include "mozilla/Attributes.h"
> >  
> > -#if defined(__cplusplus)
> > -#include "mozilla/Attributes.h" // for MOZ_HAVE_CXX11_STRONG_ENUMS
> > -#endif
> 
> Why are you removing this?

Because it's included unconditionally two lines earlier :)
Assignee

Comment 3

7 years ago
Posted patch fixSplinter Review
Assignee: nobody → jacek
Attachment #671384 - Attachment is obsolete: true
Attachment #671427 - Flags: review?(ehsan)

Comment 4

7 years ago
(In reply to Jacek Caban from comment #2)
> (In reply to Ehsan Akhgari [:ehsan] from comment #1)
> > Comment on attachment 671384 [details] [diff] [review]
> > fix v1.0
> > 
> > Review of attachment 671384 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Couldn't you just #undef ERROR if it had been defined previously?  That
> > seems like a much simpler and more robust fix.
> 
> Sure I could, but I consider avoiding collisions cleaner than hacking around
> them. I will prepare a new patch.

That's true, but we cannot really avoid doing that since whatever name we pick might also be used by some system header somewhere... And ERROR is a shorter name.  :-)

> > ::: xpcom/base/nsError.h
> > @@ -12,5 @@
> > >  #include "mozilla/Attributes.h"
> > >  
> > > -#if defined(__cplusplus)
> > > -#include "mozilla/Attributes.h" // for MOZ_HAVE_CXX11_STRONG_ENUMS
> > > -#endif
> > 
> > Why are you removing this?
> 
> Because it's included unconditionally two lines earlier :)

lol, indeed it is!

Updated

7 years ago
Attachment #671427 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/436644e80e20
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.