259325 - build error on OpenBSD

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[David]

User-Agent:       Mozilla/5.0 (X11; U; OpenBSD i386; en-US; rv:1.7) Gecko/20040710 Firefox/0.9.2
Build Identifier: FIREFOX_0_10_RELEASE

It is neither safe nor necessary to include stdbool.h in C++ code on OpenBSD.

Reproducible: Always
Steps to Reproduce:
1. Log into OpenBSD machine
2. cvs -d :pserver:[e-mail]:/cvsroot \
        co -rFIREFOX_0_10_RELEASE -f mozilla/client.mk
3. cd mozilla
4. gmake -f client.mk

Actual Results:  
compilation died with syntax error on false

Expected Results:  
Index: modules/plugin/base/public/nptypes.h
===================================================================
RCS file: /cvsroot/mozilla/modules/plugin/base/public/nptypes.h,v
retrieving revision 3.19.4.4
diff -u -r3.19.4.4 nptypes.h
--- modules/plugin/base/public/nptypes.h        1 Sep 2004 16:59:55 -0000      
3.19.4.4
+++ modules/plugin/base/public/nptypes.h        14 Sep 2004 16:35:37 -0000
@@ -78,8 +78,10 @@
     typedef int bool;
   #endif
   #else /* OPENBSD is defined, so use its bool */
+  #if !defined(__cplusplus)
     #include <stdbool.h>
   #endif
+  #endif
   #else
   /*
    * FreeBSD defines uint32_t and bool.

Comment 1

[David H. Gutteridge]

This is also relevant to NetBSD.  stdbool.h on releases older than 2.0BETA or
CURRENT from late May is not correct for C++ compilation, also some older
releases may not even have it present.  In my case compilation failed until I
updated my copy of stdbool.h.  With the 2.0 formal release, it will be safe to
include this header, but as the original reporter notes, it's unnecessary when
being compiled as C++.

I'm not sure it's safe to assume an unspecified OS ships with stdbool.h, as
currently happens in nptypes.h.  For instance, I don't see any specific code in
the file for HP-UX, and I believe older versions of it don't ship with
stdbool.h.  (It appears to have been provided as a patch by HP for 11.00 and 11.11.)

Comment 2

[Jim Rees]

I can confirm this on OpenBSD 3.3. It also works to just typedef Bool and not
include stdbool.h at all.

There is too much spaghetti in this code.

Comment 3

[Jim Rees]

Here is what I ended up with to build for OpenBSD 3.3 through 3.6 and
FreeBSD 4.x through 5.3:

Index: modules/plugin/base/public/nptypes.h
===================================================================
RCS file: /cvsroot/mozilla/modules/plugin/base/public/nptypes.h,v
retrieving revision 3.19.4.4
diff -u -r3.19.4.4 nptypes.h
--- modules/plugin/base/public/nptypes.h	1 Sep 2004 16:59:55 -0000	3.19.4.4
+++ modules/plugin/base/public/nptypes.h	23 Nov 2004 21:54:43 -0000
@@ -58,7 +58,7 @@
   #ifndef __cplusplus
     typedef int bool;
   #endif
-#elif defined(bsdi) || defined(FREEBSD) || defined(OPENBSD)
+#elif defined(bsdi) || defined(FREEBSD)
   /*
    * BSD/OS, FreeBSD, and OpenBSD ship sys/types.h that define int32_t and 
    * u_int32_t.
@@ -87,6 +87,13 @@
     #include <inttypes.h>
     #include <stdbool.h>
   #endif
+#elif defined(OPENBSD)
+  #include <sys/types.h>
+
+  typedef u_int32_t uint32_t;
+  #if !defined(__cplusplus)
+    typedef int bool;
+  #endif
 #elif defined(BEOS)
   #include <inttypes.h>
 #else

Comment 4

[Andrew Schultz]

==> plugins

Comment 5

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Patch in comment #3 WFM, will attach & request review in a moment...

Comment 6

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: patch

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 183456 [details] [diff] [review]
patch

sr=jst

Comment 8

[Andrew Schultz]

Attachment: less spaghetti (Checked in)

AFAICT this is completely equivalent code with less spaghetti (instead of more)

Comment 9

[Andrew Schultz]

Comment on attachment 183464 [details] [diff] [review]
less spaghetti (Checked in)

this built successfully on CTho's machine.

Comment 10

[Andrew Schultz]

stealing bug

Comment 11

[Jim Rees]

This builds for me on OpenBSD 3.6. I'm not set up right now to test on other
releases but since it's functionally equivalent to my patch, and looks better, I
vote to commit and mark this bug RESOLVED.

Comment 12

[Johnny Stenback (:jst)]

Comment on attachment 183464 [details] [diff] [review]
less spaghetti (Checked in)

sr=jst

Comment 13

[Andrew Schultz]

Comment on attachment 183464 [details] [diff] [review]
less spaghetti (Checked in)

simple (OpenBSD) porting fix

Comment 14

[David H. Gutteridge]

My two cents: I'd also recommend the following patch be added to the "all other
OSes" block, this was causing build failures in NetBSD, and may also on other OSes.

--- nptypes.h~	Sun May 15 00:06:05 2005
+++ nptypes.h	Sun May 15 00:07:01 2005
@@ -97,13 +97,15 @@
    */
   #include <stdint.h>
 
-  #if !defined(__GNUC__) || (__GNUC__ > 2 || __GNUC_MINOR__ > 95)
-    #include <stdbool.h>
-  #else
-    /*
-     * GCC 2.91 can't deal with a typedef for bool, but a #define
-     * works.
-     */
-    #define bool int
+  #ifndef __cplusplus
+    #if !defined(__GNUC__) || (__GNUC__ > 2 || __GNUC_MINOR__ > 95)
+      #include <stdbool.h>
+    #else
+      /*
+       * GCC 2.91 can't deal with a typedef for bool, but a #define
+       * works.
+       */
+      #define bool int
+    #endif
   #endif
 #endif

Comment 15

[Asa Dotzler [:asa]]

Comment on attachment 183464 [details] [diff] [review]
less spaghetti (Checked in)

a=asa

Comment 16

[Ian Neal]

Comment on attachment 183464 [details] [diff] [review]
less spaghetti (Checked in)

Checking in nptypes.h;
new revision: 3.24; previous revision: 3.23
done

Comment 17

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Can this bug be resolved?

Comment 18

[Jim Rees]

As I said in Comment #11 the patch fixes the bug, and is in the tree. I don't
think it has made it in to the Firefox branch yet but as far as I'm concerned
this is resolved.

If NetBSD or other OSes have a similar problem it should be opened as a separate
bug.

Comment 19

[Andrew Schultz]

resolving FIXED...

David: can you file a new bug and attach your patch (and request review)?  I
think the reason the current code is not the way you've proposed is that some
(very) old compilers did not have an internal bool type.  I don't if we care
about those compilers anymore or not.

Also, there is no "Firefox" branch... there is an aviary branch (for 1.0.x
releases), but gecko core (including plugins) should be the same in 1.7 and the
aviary branch.  My patch only landed on the trunk.

Comment 20

[David H. Gutteridge]

I've opened a new bug for the NetBSD/other issue, it's 297832