diff mbox

AIX net/if_arp.h include fix for struct fc_softc

Message ID 20110606041634.GA6074@tw2.lan
State New
Headers show

Commit Message

Peter O'Gorman June 6, 2011, 4:16 a.m. UTC
Hi,

We ran across an issue with qt-4.7 built with gcc-4.4 on AIX 5.2, 5.3,
6.1, and 7.1 where some static constructors were not being called. It
turned out to be a header file issue, see, for example,
https://www.ibm.com/developerworks/forums/thread.jspa?threadID=211873&tstart=-2

Using fixincludes to fix the header allows us to build a working qt.

The erroneous struct declaration is:
struct  fc_softc {
        struct arpcom   fc_ac;              /* FCS common part             */
        struct ndd    *nddp;                /* returned from NS            */
        int(*efcnet_arp_fct)
             (struct ndd *, struct mbuf *); /* efcnet_arp function address */
} *fc_softc ;

when fixed it becomes:
typedef struct _fc_softc {                   
        struct arpcom   fc_ac;              /* FCS common part             */
        struct ndd    *nddp;                /* returned from NS            */
        int(*efcnet_arp_fct)
             (struct ndd *, struct mbuf *); /* efcnet_arp function address */
} *fc_softc ;

David, do you have any idea if this is what it's supposed to be?

Ok for trunk?

Peter

Comments

Bruce Korb June 6, 2011, 2:11 p.m. UTC | #1
On 06/05/11 21:16, Peter O'Gorman wrote:
> Ok for trunk?
>
> Peter

> Index: ChangeLog
> ===================================================================
> --- ChangeLog	(revision 174678)
> 2011-??-??  Peter O'Gorman  <pogma@thewrittenword.com>
>
> 	* inclhack.def (aix_net_if_arp): New fix.
> 	* fixincl.x: Regenerate.
> 	* tests/base/net/if_arp.h [AIX_NET_IF_ARP_CHECK]: New test.
>
> Index: inclhack.def
> ===================================================================
> --- inclhack.def	(revision 174678)
> +++ inclhack.def	(working copy)
> @@ -369,6 +369,19 @@
>      test_text = "#define _Complex_I	__I\n";
>  };
>
> +/*
> + * net/if_arp.h defines a variable fc_softc instead of adding a
> + * typedef for the struct on AIX 5.2, 5.3, 6.1 and 7.1
> + */
> +fix = {
> +    hackname  = aix_net_if_arp;
> +    mach      = "*-*-aix*";
> +    files     = "net/if_arp.h";
> +    select    = "^struct  fc_softc \\{";
> +    c_fix     = format;
> +    c_fix_arg = "typedef struct _fc_softc {";
> +    test_text = "struct  fc_softc {";
> +};
>
>  /*
>   *  pthread.h on AIX 4.3.3 tries to define a macro without whitspace

Please be kind enough to make the test text sample be a complete
struct, even if it only has one "int" field.  Aesthetics.  Sorry.
The change log showed a change for tests/base/net/if_arp.h, but
the patch did not.

Otherwise, fine by me.
David Edelsohn June 6, 2011, 3:05 p.m. UTC | #2
On Mon, Jun 6, 2011 at 12:16 AM, Peter O'Gorman
<pogma@thewrittenword.com> wrote:
> Hi,
>
> We ran across an issue with qt-4.7 built with gcc-4.4 on AIX 5.2, 5.3,
> 6.1, and 7.1 where some static constructors were not being called. It
> turned out to be a header file issue, see, for example,
> https://www.ibm.com/developerworks/forums/thread.jspa?threadID=211873&tstart=-2
>
> Using fixincludes to fix the header allows us to build a working qt.
>
> The erroneous struct declaration is:
> struct  fc_softc {
>        struct arpcom   fc_ac;              /* FCS common part             */
>        struct ndd    *nddp;                /* returned from NS            */
>        int(*efcnet_arp_fct)
>             (struct ndd *, struct mbuf *); /* efcnet_arp function address */
> } *fc_softc ;
>
> when fixed it becomes:
> typedef struct _fc_softc {
>        struct arpcom   fc_ac;              /* FCS common part             */
>        struct ndd    *nddp;                /* returned from NS            */
>        int(*efcnet_arp_fct)
>             (struct ndd *, struct mbuf *); /* efcnet_arp function address */
> } *fc_softc ;
>
> David, do you have any idea if this is what it's supposed to be?
>
> Ok for trunk?

The header certainly does not make sense as is and does not follow AIX
header file conventions.  typedef is the only thing that makes sense,
which was confirmed by some other AIX developer with whom I checked.
(I would have expected fc_softc_t as well if it was intended as a
typedef, but that's a separate issue.)

This change is okay.

I will try to find the AIX header owners to fix the problem as well.

Thanks, David
Peter O'Gorman June 6, 2011, 4:16 p.m. UTC | #3
On Mon, Jun 06, 2011 at 11:05:25AM -0400, David Edelsohn wrote:
> > David, do you have any idea if this is what it's supposed to be?
> >
> > Ok for trunk?
> 
> The header certainly does not make sense as is and does not follow AIX
> header file conventions.  typedef is the only thing that makes sense,
> which was confirmed by some other AIX developer with whom I checked.
> (I would have expected fc_softc_t as well if it was intended as a
> typedef, but that's a separate issue.)
> 
> This change is okay.

Thanks, committed in r174707 with Bruce's change to the test.

> 
> I will try to find the AIX header owners to fix the problem as well.

That would be great.

Peter
diff mbox

Patch

Index: inclhack.def
===================================================================
--- inclhack.def	(revision 174678)
+++ inclhack.def	(working copy)
@@ -369,6 +369,19 @@ 
     test_text = "#define _Complex_I	__I\n";
 };
 
+/*
+ * net/if_arp.h defines a variable fc_softc instead of adding a
+ * typedef for the struct on AIX 5.2, 5.3, 6.1 and 7.1
+ */
+fix = {
+    hackname  = aix_net_if_arp;
+    mach      = "*-*-aix*";
+    files     = "net/if_arp.h";
+    select    = "^struct  fc_softc \\{";
+    c_fix     = format;
+    c_fix_arg = "typedef struct _fc_softc {";
+    test_text = "struct  fc_softc {";
+};
 
 /*
  *  pthread.h on AIX 4.3.3 tries to define a macro without whitspace