79282 - g++ 2.95.2 does not define _LANGUAGE_C_PLUS_PLUS required for <signal.h>

Status: VERIFIED FIXED

(SeaMonkey :: Build Config, defect, P3)

Description

[John Vandenberg]

signal() has a different specification depending on whether it is included from 
C or C++ .  signal.h uses #ifdef _LANGUAGE_C and #ifdef _LANGUAGE_C_PLUS_PLUS 
to determine this.  When compiling with g++, I need to #define 
_LANGUAGE_C_PLUS_PLUS in netwerk/test/TestOverlappedIO.cpp and 
netwerk/test/TestSocketIO.cpp .

% man signal
...
C SYNOPSIS
     #include <signal.h>

     C:
       void (*signal (int sig, void (*func)()))();
     C++:
       void (*signal (int sig, void (*func)(int)))(int);
...
%

Comment 1

[John Vandenberg]

Attachment: Ugly patch to TestOverlappedIO.cpp

Comment 2

[cls]

I'd rather stick that define in the irix section of configure.in .  Other cases
may pop up that need this fix as well and I'm not sure what effect, if any, this
would have on non-irix systems.

Comment 3

[John Vandenberg]

Agreed.  I am going thru with workarounds until I can go no more.  Then ill 
clean my objdir, come back and clean up and test these bugs, unless of course 
you want to provide the fix? :)

Comment 4

[John Vandenberg]

Attachment: Patch to netwerk/test/Makefile.in

Comment 5

[John Vandenberg]

While this isnt configure.in, so far this problem has not affected the rest of 
the build.  It has so far taken me 6 hours to build using gnu compilers as 
opposed to about 2.5 hours using MIPSpro, so I am reluctant to rebuild it all 
in order to test that adding the define globally does not break something 
else.  I have tested this patch does not break the MIPSpro build.

Comment 6

[cls]

Attachment: define _LANGUAGE_C_PLUS_PLUS in configure.in for irix gcc builds

Comment 7

[Asa Dotzler [:asa]]

has this been reviewed? 

Comment 8

[John Mark Vandenberg]

I have verified Chris's patch works for me if that is of any use.

Comment 9

[John Mark Vandenberg]

I am not sure, but I believe there is a problem with the change to 
configure.in ... and am suprised that my build worked.  

AC_DEFINE will define _LANGUAGE_C_PLUS_PLUS for both c & c++, which would 
result in signals being broken in c instead of c++.

So the configure.in should have something to the effect of CXXFLAGS += -
D_LANGUAGE_C_PLUS_PLUS.

Comment 10

[cls]

Attachment: configure.in patch - take 2

Comment 11

[John Mark Vandenberg]

Given that this define is required on IRIX, I have opened an SGI internal bug 
(825388), to have g++ internally define it.

Comment 12

[John Mark Vandenberg]

This issue is now also a GNU bug: c++/3047

g++ does define _LANGUAGE_C_PLUS_PLUS for .cc .cxx and .C , but obviously 
not .cpp

Comment 13

[John Vandenberg]

Can this be moved forward to 0.9.2 as the fix is in hand, and this is the only 
remaining blocker for IRIX/gcc build.

Comment 14

[cls]

Target milestone doesn't matter that much but I will need a r= on the new patch
before drivers will accept it.

Comment 15

[John Vandenberg]

I am not familiar with the preferred method of requesting review; should I 
email the person personally, or add them to the CC, the latter seeming a little 
rude?

Comment 16

[cls]

I think either way is fairly common practice.  Since this is a considered a port
specific branch, anyone who is familiar with the port and can verify that the
patch does work like it should can give a r=.  Ports really don't have the
owner/peer restrictions like other modules do.

Comment 17

[John Vandenberg]

Wan-Teh, could you please review this patch for the IRIX/gcc port.

Comment 18

[Wan-Teh Chang]

r=wtc.

Comment 19

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 20

[cls]

Patch has been checked in.

Comment 21

[Jon Granrose]

verified.