Patchwork AIX net/if_arp.h include fix for struct fc_softc

login
register
mail settings
Submitter Peter O'Gorman
Date June 6, 2011, 4:16 a.m.
Message ID <20110606041634.GA6074@tw2.lan>
Download mbox | patch
Permalink /patch/98799/
State New
Headers show

Comments

Peter O'Gorman - June 6, 2011, 4:16 a.m.
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
Bruce Korb - June 6, 2011, 2:11 p.m.
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.
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.
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

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