Bug 13383

Summary: sys/netinet/in.h violates C++ spec.
Product: Base System Reporter: Justin T. Gibbs <gibbs>
Component: binAssignee: Jacques Vidrine <nectar>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: 4.0-CURRENT   
Hardware: Any   
OS: Any   

Description Justin T. Gibbs 1999-08-25 23:00:01 UTC
	Compiling such a program generates the error:

	ANSI C++ forbids data member `ip_opts' with same name as
	enclosing class

Fix: 

Rename ip_opts in struct ip_opts to something else.  I have
	no idea what the impact of this change is.
How-To-Repeat: 
	Compile a C++ application that makes use of netinet/in.h.
Comment 1 Jacques Vidrine 1999-10-06 05:13:09 UTC
Can you send example code?  A brief test here doesn't turn up an
error.  What specific version of gcc 2.95 are you using?  Perhaps this
was fixed in 2.95.1?

Though ANSI C++ may forbid such a construct, a conforming C++ compiler
should accept legal ANSI C when it is specified as such (using extern
"C" or whatever).

--- test.cc ---
#include <sys/types.h>
#include <netinet/in.h>
#include <string.h>

void 
foo()
{
	struct ip_opts my_ip_opts;
	memset(my_ip_opts.ip_opts, 0, sizeof(my_ip_opts.ip_opts));
}
--- test.cc ---

% g++295 -Wall -c test.cc
% echo $?
0
% g++295 -v
Reading specs from /opt/lib/gcc-lib/i386-portbld-freebsd3.3/2.95.1/specs
gcc version 2.95.1 19990816 (release)

Jacques Vidrine / n@nectar.com / nectar@FreeBSD.org
Comment 2 Thomas David Rivers 1999-10-06 12:01:46 UTC
>  
>  Though ANSI C++ may forbid such a construct, a conforming C++ compiler
>  should accept legal ANSI C when it is specified as such (using extern
>  "C" or whatever).

 I believe (I could be wrong) that all extern "C" does is affect
 the linkage of functions declared in the extern "C" block.  That is,
 functions declared in an extern "C" block will have a `C signature'.
 I believe that's all it means.

 It does not mean that the source within the block can violate C++ standards.

 I have a .PDF version of the C++ standard here that I can check
 later.

	- Dave Rivers -
Comment 3 Jacques Vidrine 1999-10-06 17:47:42 UTC
On 6 October 1999 at 7:01, Thomas David Rivers <rivers@dignus.com> wrote:
>  I believe (I could be wrong) that all extern "C" does is affect
>  the linkage of functions declared in the extern "C" block.  
[snip]

I think you are right, that's why I hedged with ``whatever''.  The
system C headers have to be handled in some manner by the C++
compiler...  the mechanism is probably ``up to the implementation''.
 
>  I have a .PDF version of the C++ standard here that I can check
>  later.

That would be great if you can look, just to satisfy curiosity.  But
like I mentioned, this issue seems to have been resolved with gcc
2.95.1, though I could have missed something that Justin found.

Jacques Vidrine / n@nectar.com / nectar@FreeBSD.org
Comment 4 Thomas David Rivers 1999-10-06 18:19:13 UTC
> 
> On 6 October 1999 at 7:01, Thomas David Rivers <rivers@dignus.com> wrote:
> >  I believe (I could be wrong) that all extern "C" does is affect
> >  the linkage of functions declared in the extern "C" block.  
> [snip]
> 
> I think you are right, that's why I hedged with ``whatever''.  The
> system C headers have to be handled in some manner by the C++
> compiler...  the mechanism is probably ``up to the implementation''.
>  
> >  I have a .PDF version of the C++ standard here that I can check
> >  later.
> 
> That would be great if you can look, just to satisfy curiosity.  But
> like I mentioned, this issue seems to have been resolved with gcc
> 2.95.1, though I could have missed something that Justin found.
> 

 I think gcc 2.95.1 "improved" it's standards conformance.  It is
 now diagnosing errors previous versions didn't.

 Could that be the issue?

	- Dave R. -
Comment 5 Jacques Vidrine 1999-10-06 19:02:39 UTC
On 6 October 1999 at 13:19, Thomas David Rivers <rivers@dignus.com> wrote:
>  I think gcc 2.95.1 "improved" it's standards conformance.  It is
>  now diagnosing errors previous versions didn't.
> 
>  Could that be the issue?

No, see the text of this PR.  To summarize:

g++ version          gives error
2.95			yes (according to Justin)
2.7.2.3			no (test case in PR)
2.91.66			no (test case in PR)
2.95.1			no (test case in PR)

I don't have a position on which is CORRECT, but obviously the latter
is more useful.

Further, perhaps there is some confusion with regards to the compiler
version Justin is using, or perhaps there is a test case that Justin
has that produces the error.

Jacques Vidrine / n@nectar.com / nectar@FreeBSD.org
Comment 6 gibbs 1999-10-06 19:23:29 UTC
>On 6 October 1999 at 13:19, Thomas David Rivers <rivers@dignus.com> wrote:
>>  I think gcc 2.95.1 "improved" it's standards conformance.  It is
>>  now diagnosing errors previous versions didn't.
>> 
>>  Could that be the issue?
>
>No, see the text of this PR.  To summarize:
>
>g++ version          gives error
>2.95			yes (according to Justin)
>2.7.2.3			no (test case in PR)
>2.91.66			no (test case in PR)
>2.95.1			no (test case in PR)
>
>I don't have a position on which is CORRECT, but obviously the latter
>is more useful.
>
>Further, perhaps there is some confusion with regards to the compiler
>version Justin is using, or perhaps there is a test case that Justin
>has that produces the error.
>
>Jacques Vidrine / n@nectar.com / nectar@FreeBSD.org

[waterboy::pluto]$ /build/gibbs/local/bin/g++ foo.cc -o foo
foo.cc:6: redefinition of `struct ip_opts'
/usr/include/netinet/in.h:292: previous definition here
foo.cc:8: ANSI C++ forbids data member `ip_opts' with same name as enclosing class
[waterboy::pluto]$ /build/gibbs/local/bin/g++ -Wall foo.cc -o foo
foo.cc:6: redefinition of `struct ip_opts'
/usr/include/netinet/in.h:292: previous definition here
foo.cc:8: ANSI C++ forbids data member `ip_opts' with same name as enclosing class
[waterboy::pluto]$ /build/gibbs/local/bin/g++ -v
Reading specs from /build/gibbs/local/lib/gcc-lib/i386-portbld-freebsd3.2/2.95.1/specs
gcc version 2.95.1 19990816 (release)

in.h does not have any C++ protection, so if in.h has invalid C++ in it,
the compiler is right to complain.

--
Justin
Comment 7 Dmitrij Tejblum 1999-10-06 22:19:32 UTC
OpenBSD fixed <netinet/in.h> and their commit log says the fix follow
OSF/1. IMO we should follow them.

Dima
Comment 8 Jacques Vidrine 1999-10-08 01:18:55 UTC
On Thu, 07 Oct 1999 13:18:10 -0600, "Justin T. Gibbs" <gibbs@caspian.plutotech.com> wrote:
[snip]
> This certainly will show the error, my complicated C++ program shows the
> error, but your example does not:
> foo.cc:
> #include <sys/types.h>
> #include "foo.h"
> 
> int
> main()
> {
>         return 1;
> }
> 
> foo.h:
> #include <sys/cdefs.h>
> 
> __BEGIN_DECLS   /* extern C protection has no effect */
> struct in_addr {
>         u_int32_t s_addr;
> };
> 
> struct ip_opts {
>         struct  in_addr ip_dst;         /* first hop, 0 w/o src rt */
>         char    ip_opts[40];            /* actually variable in size */
> };
> __END_DECLS 

Ah ha!

Change "foo.h" to <foo.h> and use ``g++ -I$PWD [...]'' -- the
compilation will complete successfully and without warnings.

GCC is treating ``system'' headers specially.  Seems reasonable to me.

As far as I am concerned, there are two alternatives we can follow:

1) Do nothing, all versions of gcc and egcs seem to handle the situation
   in a reasonable and useful manner (aka if it isn't broke, don't fix it).
2) Adopt the change that OSF/1 and OpenBSD have (see patch at the end of
   this message).

I'm going to research the C++ standards I can get my hands on this
weekend to determine if the current behavior is conformant or not.
If it is not conformant, I will do (2).

--- in.h.orig   Thu Oct  7 19:17:26 1999
+++ in.h        Thu Oct  7 19:18:28 1999
@@ -289,7 +289,11 @@
  */
 struct ip_opts {
        struct  in_addr ip_dst;         /* first hop, 0 w/o src rt */
+#if defined(__cplusplus)
+       char    Ip_opts[40];            /* the member name must be different */
+#else
        char    ip_opts[40];            /* actually variable in size */
+#endif
 };
 /*

Jacques Vidrine / n@nectar.com / nectar@FreeBSD.org
Comment 9 gibbs 1999-10-08 02:07:18 UTC
>Ah ha!
>
>Change "foo.h" to <foo.h> and use ``g++ -I$PWD [...]'' -- the
>compilation will complete successfully and without warnings.

This doesn't tell the whole story.  Look at this:

[waterboy::libmci]$ make Mothership.o
/build/gibbs/local/bin/g++  -O -pipe -I/usr/local/tcltk/include -I. -I../include
 -I../../../src/sys -pipe -DPOOLING_ENABLED -I../../include -I../../include/expo
rted -g -Wall -DDEBUG=0 -c Mothership.cc -o Mothership.o
In file included from Mothership.cc:19:
../../../src/sys/netinet/in.h:291: ANSI C++ forbids data member `ip_opts' with s
ame name as enclosing class
*** Error code 1

Stop.
[waterboy::libmci]$ grep netinet Mothership.cc
#include <netinet/in.h>
#include <netinet/tcp.h> 

This was the original case of the error that caused me to report it.

--
Justin
Comment 10 Jacques Vidrine 2000-03-30 21:06:49 UTC
I believe this PR should be closed with no action, because:

  + Recent activities by the C++ Standards working group indicates
    that though the current version of the standard prohibits structures
    like ip_opts, this was a mistake made in 1996 that will be corrected
    in a future version of the standard (excerpt below, snarfed from
    http://wwwold.dkuug.dk/JTC1/SC22/WG21/docs/core8.htm).

  + egcs allows this construction when it is encountered within
    ``extern "C"'' .  This is non-standard, but practical.  This
    feature was added by Jason Merrill <jason@cygnus.com> in revision
    1.282 of gcc/cp/decl.c on 12 Dec 1998.

===== begin excerpt =====
Document number:  J16/99-0032 = WG21 N1208
           Date:  23 February, 2000       
        Project:  Programming Language C++
      Reference:  ISO/IEC IS 14882:1998(E)
       Reply to:  William M. Miller       
                  wmm@fastdial.net        


            C++ Standard Core Language Issues List, Revision 9             

80. Class members with same name as class

Section: 9.2 class.mem    Status: review   Submitter: Jason Merrill   Date:
5 Dec 1998

Between the May '96 and September '96 working papers, the text in 9.2
class.mem paragraph 13:
   
    If T is the name of a class, then each of the following shall have a
    name different from T:
      +  every static data member of class T;

was changed by removing the word 'static'. Looking over the meeting minutes
from Stockholm, none of the proposals seem to include this change, which
breaks C compatibility and is not mentioned in the compatibility annex. Was
this change actually voted in by the committee?

Specifically, this breaks /usr/include/netinet/in.h under Linux, in which
"struct ip_opts" shares its name with one of its members.

Proposed resolution (10/99):
   
 1. Change the first bullet of 9.2 class.mem paragraph 13 to say
      +  every static data member of class T;
 2. Add another paragraph before 9.2 class.mem paragraph 14, reading
        In addition, if class T has a user-declared constructor (12.1
        class.ctor ), every nonstatic data member of class T shall have a
        name different from T.
=====  end excerpt  =====
-- 
Jacques Vidrine / n@nectar.com / nectar@FreeBSD.org
Comment 11 Jacques Vidrine freebsd_committer freebsd_triage 2000-03-30 21:44:56 UTC
Responsible Changed
From-To: freebsd-bugs->nectar

I already invested too much time in this :-)  so I might as well 
follow through. 
Comment 12 Jacques Vidrine freebsd_committer freebsd_triage 2000-06-15 18:59:23 UTC
State Changed
From-To: open->closed

By request of PETA, prevent beating of dead horse.