Closed Bug 259325 Opened 20 years ago Closed 19 years ago

build error on OpenBSD

Categories

(Core Graveyard :: Plug-ins, defect)

All
OpenBSD
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.8beta2

People

(Reporter: nospam-bugzilla, Assigned: ajschult784)

Details

Attachments

(2 files)

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:anonymous@cvs-mirror.mozilla.org:/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.
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.)
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.
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
==> plugins
Assignee: bryner → nobody
Component: Build Config → Plug-ins
Product: Firefox → Core
QA Contact: asa → plugins
Version: unspecified → 1.7 Branch
Patch in comment #3 WFM, will attach & request review in a moment...
Assignee: nobody → cst
Status: UNCONFIRMED → NEW
Ever confirmed: true
Target Milestone: --- → mozilla1.8beta2
Comment on attachment 183456 [details] [diff] [review]
patch

sr=jst
Attachment #183456 - Flags: superreview+
AFAICT this is completely equivalent code with less spaghetti (instead of more)
Comment on attachment 183464 [details] [diff] [review]
less spaghetti (Checked in)

this built successfully on CTho's machine.
Attachment #183464 - Flags: superreview?(jst)
Attachment #183464 - Flags: review?(benjamin)
stealing bug
Assignee: cst → ajschult
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.
Attachment #183464 - Flags: review?(benjamin) → review+
Comment on attachment 183464 [details] [diff] [review]
less spaghetti (Checked in)

sr=jst
Attachment #183464 - Flags: superreview?(jst) → superreview+
Comment on attachment 183464 [details] [diff] [review]
less spaghetti (Checked in)

simple (OpenBSD) porting fix
Attachment #183464 - Flags: approval1.8b2?
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
Attachment #183456 - Flags: review?(benjamin)
Comment on attachment 183464 [details] [diff] [review]
less spaghetti (Checked in)

a=asa
Attachment #183464 - Flags: approval1.8b2? → approval1.8b2+
Comment on attachment 183464 [details] [diff] [review]
less spaghetti (Checked in)

Checking in nptypes.h;
new revision: 3.24; previous revision: 3.23
done
Attachment #183464 - Attachment description: less spaghetti → less spaghetti (Checked in)
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.
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.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Version: 1.7 Branch → Trunk
I've opened a new bug for the NetBSD/other issue, it's 297832
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: