Closed
Bug 801589
Opened 12 years ago
Closed 12 years ago
Avoid redefining ERROR in ErrorList.h when wingdi.h is also included
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla19
People
(Reporter: jacek, Assigned: jacek)
References
Details
Attachments
(1 file, 1 obsolete file)
|
1.44 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
wingdi.h (a code Windows SDK header) defines ERROR to 0.
Attachment #671384 -
Flags: review?(ehsan)
|
Assignee | |
Updated•12 years ago
|
Blocks: nsresult-enum-class
|
||
Comment 1•12 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•12 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•12 years ago
|
||
Assignee: nobody → jacek
Attachment #671384 -
Attachment is obsolete: true
Attachment #671427 -
Flags: review?(ehsan)
|
|
||
Comment 4•12 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•12 years ago
|
Attachment #671427 -
Flags: review?(ehsan) → review+
|
|
Assignee | |
Comment 5•12 years ago
|
||
|
||
Comment 6•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 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.
Description
•