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

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Jacek Caban]

Attachment: fix v1.0

wingdi.h (a code Windows SDK header) defines ERROR to 0.

Comment 1

[(no longer active)]

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?

Comment 2

[Jacek Caban]

(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 :)

Comment 3

[Jacek Caban]

Attachment: fix

Comment 4

[(no longer active)]

(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!

Comment 5

[Jacek Caban]

https://hg.mozilla.org/integration/mozilla-inbound/rev/436644e80e20

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/436644e80e20