Make security compile for BeOS/Haiku gcc4

NEW
Unassigned

Status

11 years ago
10 years ago

People

(Reporter: thesuckiestemail, Unassigned)

Tracking

(Blocks: 1 bug)

3.11.5
x86
BeOS

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

1.53 KB, patch
thesuckiestemail
: review?
wtc
Details | Diff | Splinter Review
(Reporter)

Description

11 years ago
Created attachment 306127 [details] [diff] [review]
patch against 1.8 tree

Fixing so that the security (/nss) subdir compiles for BeOS/Haiku and gcc4:
* Renaming a var called bool to boolvar
* Remove the be/ prefix from a BeOS-header, to the more common way.
(Reporter)

Comment 1

11 years ago
Comment on attachment 306127 [details] [diff] [review]
patch against 1.8 tree

r?

Need help landing this, probably on both 1.8 and trunk.
Attachment #306127 - Flags: review?(wtc)
NSS is not on the 1.8 branch, the 1.8 branch builds use tagged NSS releases. You'll need this patch in a branch NSS, the NSS team tags a release, and then maybe we upgrade to a new release (but we rarely, rarely do).
Assignee: dveditz → nobody
Component: Security → Libraries
Product: Core → NSS
QA Contact: toolkit → libraries
Version: 1.8 Branch → 3.11.5
(Reporter)

Comment 3

11 years ago
Ok, I might try to find out what compiler flag makes gcc4 say bool is a reserved word then, as it seems no other platforms have that problem. The other fix is just more of a annoyance.

Might be a better/faster way.
Comment on attachment 306127 [details] [diff] [review]
patch against 1.8 tree

Review comments:
1. The patch for file lib/dev/ckhelper.c seems fine to me.
I'd be happy to commit it on the NSS 3.11.x branch, but
doing so is not likely to help, because mozilla is only taking
code from a tag frozen in time long long ago in a galaxy far far ...

2. The patch for lib/freebl/unix_rand.c raises this concern.
The existing line, which your patch changes, was set by some BEOS
programmer.  Evidently it worked for one BEOS programmer but not 
for another.  Perhaps if we change it, it will fix the problem for
you, and break builds for other BEOS programmers.  So, what the 
NSS teams needs, to consider this part of the patch, is some 
indication of widespread consensus that this is the "right" fix 
for BEOS.

>-#include <be/kernel/OS.h>
>+#include <kernel/OS.h>

Also, this file is part of the tree that Mozilla most wants to preserve
and least wants to change, so even if we commit this fix on the NSS 3.11.x
branch, I think Mozilla is very unlikely to "take" this fix.

Comment 5

11 years ago
These files use 'bool':
http://lxr.mozilla.org/security/search?string=+bool%3B

They should be reviewed and fixed if appropriate.
(Reporter)

Comment 7

11 years ago
(In reply to comment #4)
> (From update of attachment 306127 [details] [diff] [review])

> 2. The patch for lib/freebl/unix_rand.c raises this concern.
> The existing line, which your patch changes, was set by some BEOS
> programmer.  Evidently it worked for one BEOS programmer but not 
> for another.  Perhaps if we change it, it will fix the problem for
> you, and break builds for other BEOS programmers.  So, what the 
> NSS teams needs, to consider this part of the patch, is some 
> indication of widespread consensus that this is the "right" fix 
> for BEOS.

It doesn't break anything. The BeOS gcc-compiler has a env variable called BEINCLUDES. It includes a paths for includes with paths to a root and to every subdir under. This  becomes painful for our new gcc4 as it doesn't have anything similar, so you have to have the same header in be/kernel/OS.h kernel/OS.h and OS.h. This was just to simplify, so it can safely be skipped, although it will stand out compared to all other BeOS-headers. 

I suggest we focus on bool's alone then.

Comment 8

11 years ago
(In reply to comment #3)

I agree that we should find out what compiler flag makes gcc4 say
bool is a reserved word.  We can compile that file with gcc4 on many
other platforms.
I would oppose forcing bool to be treated as reserved on all platforms.
This is c code, not c++

Comment 10

11 years ago
I meant, find that compiler flag, and turn it off.

'bool' is a macro defined in <stdbool.h> in C99, so in a C file that doesn't
include <stdbool.h>, we should be able to use 'bool' as an identifier.
(Reporter)

Comment 11

11 years ago
bool is typedefed in Haiku here:
http://haiku.it.su.se:8180/source/xref/headers/os/support/SupportDefs.h#90
and is used at least by a few system-headers.

I can work around it by #undef'ing it after I've included the headers that needs it in our _beos.h NSPR-header. Do you think this is something I should report to the Haiku developers to change the handling of bool?

Comment 12

11 years ago
tqh: it seems OK for SupportDefs.h to include <stdbool.h> to provide 'bool'
to C code.  The answer to your question depends on whether we can
avoid including SupportDefs.h.  If we can avoid including SupportDefs.h,
then that's the best workaround.  If SupportDefs.h is an essential header
on BeOS, then it may make sense for it to avoid providing 'bool'.
BeOS is not a major platform, so anything it does to make porting
to BeOS hard is bad.
You need to log in before you can comment on or make changes to this bug.