550261 - Trunk can't currently build with qt, due to macro definition of malloc, realloc, etc...

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Qt, defect)

Description

[Mike Kristoffersen (:MikeK)]

The QT headers uses the names (malloc, realloc, etc) as function names in some of their classes, these names were redefined as macros in Bug 441324, the solution could be to include the QT headers first (As at least the realloc is a private function name to the class, this should be safe).

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Yes, please include the Qt headers first.

Comment 2

[Mike Kristoffersen (:MikeK)]

Attachment: Patch with the #includes in a different order

Fits:

changeset:   38933:7db1b9d406ef
tag:         tip
user:        Srirang G Doddihal <[e-mail]>
date:        Thu Mar 04 10:28:36 2010 +0100
summary:     Bug 418524 - label of "Download activity summary in the status bar" with one download should be "1" not "One". r=gavin

Comment 3

[Mike Kristoffersen (:MikeK)]

Attachment: Now without white space changes

Fits same as the previous patch

Comment 4

[Doug Turner (:dougt)]

Comment on attachment 430405 [details] [diff] [review]
Now without white space changes


>-    nsSlots* slots = CreateSlots();
>-    if (slots) {
>-      mFlagsOrSlots = reinterpret_cast<PtrBits>(slots);
>+    // Due to the QT standard header files "slots" are unfortunately a no-go
>+    // name (as it can be #defined to <nothing>) - and yes, it's a bad thing!
>+    nsSlots* newSlots = CreateSlots();

Lets just rename it.  I am not sure if a comment is really needed.  Also, have you tried:

class nsSlots* slots = CreateSlots();

No need for the extra ws here

 #include "cairo.h"
-#include <QWidget>
+

Leave a extra ws here:

  * ***** END LICENSE BLOCK ***** */
+#include <QWidget>


There are a few other areas, but basically leave a ws line after the end of the license block.

no need for a comment like this:

+// The nsToolkit.h file needs to come first as it includes qt specific headers

or this:

+// Currently it's important that the nsAppShell.h and nsWindow.h headers are
+// the first ones, as they include qt specific headers that don't respond well
+// to the definition of realloc and malloc as a macros


(basically we all know that #include files have ordering implications)

Comment 5

[Mike Kristoffersen (:MikeK)]

(In reply to comment #4)
> (From update of attachment 430405 [details] [diff] [review])
> 
> >-    nsSlots* slots = CreateSlots();
> >-    if (slots) {
> >-      mFlagsOrSlots = reinterpret_cast<PtrBits>(slots);
> >+    // Due to the QT standard header files "slots" are unfortunately a no-go
> >+    // name (as it can be #defined to <nothing>) - and yes, it's a bad thing!
> >+    nsSlots* newSlots = CreateSlots();
> 
> Lets just rename it.  I am not sure if a comment is really needed.  Also, have
> you tried:
> 
> class nsSlots* slots = CreateSlots();

No??? - What would you gain by doing that?  The preprocessor changes:

nsSlots* slots = CreateSlots();

to:

nsSlots* = CreateSlots(); 

Because there is a: #define slots
in the QT header files (like #define 'slots' to <nothing>)

> (basically we all know that #include files have ordering implications)

I'm sad to hear that our code is in that bad a shape :( - even this patch is a consequence of exactly that statement...  

Patch has been updated with the rest of the comments, will post after a sanity check.

Comment 6

[Mike Kristoffersen (:MikeK)]

Attachment: Updated with review comments

Still fits the same changelist

Comment 7

[Doug Turner (:dougt)]

http://hg.mozilla.org/mozilla-central/rev/6c5e066d074f