[ofw] [PATCH] don't include initguid in interface headers

[Fab Tillier]

Hefty, Sean wrote on Tue, 11 Jan 2011 at 17:19:40
> 
> > Index: core/winverbs/kernel/wv_driver.c
> >
> ===================================================================
> > --- core/winverbs/kernel/wv_driver.c	(revision 3068)
> > +++ core/winverbs/kernel/wv_driver.c	(working copy)
> > @@ -31,10 +31,8 @@
> >  #include <wdf.h>
> >  #include <wdmsec.h>
> >  #include <ntstatus.h>
> > -#include <initguid.h>
> >
> >  #include "index_list.c"
> > -#include <rdma/verbs.h>
> >  #include "wv_driver.h"
> >  #include "wv_ioctl.h"
> >  #include "wv_provider.h"
> > @@ -45,6 +43,10 @@
> >  #include "wv_srq.h"
> >  #include "wv_qp.h"
> >  #include "wv_ep.h"
> > +
> > +#include <initguid.h>
> > +#include <rdma/verbs.h>
> > +#include <iba\ib_cm_ifc.h>
> 
> Please keep the direction of the slashes consistent.

Will fix.  The forward slash convention dates back to the days where the code was meant to be cross-OS portable.

>  Why is this change
> needed?  I.e. initguid.h was already included before verbs.h, and why was
> ib_cm_ifc.h added?

Initguid was included towards the top of all the includes, which means any other GUID definitions from other headers would be unnecessarily defined.  This change consolidates GUID definitions after all other includes to prevent this.  I had to explicitly add ib_cm_ifc.h because its GUID was accidentally defined through some include dependency, and the code references GUID_INFINIBAND_INTERFACE_CM.

> 
> >
> >  WDF_DECLARE_CONTEXT_TYPE_WITH_NAME(WV_RDMA_DEVICE,
> WvRdmaDeviceGetContext)
> >
> > Index: core/bus/kernel/bus_pnp.c
> >
> ===================================================================
> > --- core/bus/kernel/bus_pnp.c	(revision 3068)
> > +++ core/bus/kernel/bus_pnp.c	(working copy)
> > @@ -44,12 +44,15 @@
> >  #include "bus_port_mgr.h"
> >  #include "bus_iou_mgr.h"
> >  #include "complib/cl_memory.h"
> > -#include <initguid.h>
> > -#include "iba/ib_ci_ifc.h"
> > -#include "iba/ib_cm_ifc.h"
> >  #include "al_cm_cep.h"
> >  #include "al_mgr.h"
> >  #include "bus_ev_log.h"
> > +
> > +#include <initguid.h>
> > +#include "rdma/verbs.h"
> > +#include "iba/ib_al_ifc.h"
> > +#include "iba/ib_ci_ifc.h"
> > +#include "iba/ib_cm_ifc.h"
> 
> Why was verbs.h added?

GUID_RDMA_INTERFACE_VERBS was referenced in __register_ca.
	
> 
> >  /* Interface names are generated by IoRegisterDeviceInterface. */
> > Index: inc/kernel/rdma/verbs.h
> >
> ===================================================================
> > --- inc/kernel/rdma/verbs.h	(revision 3068)
> > +++ inc/kernel/rdma/verbs.h	(working copy)
> > @@ -27,12 +27,9 @@
> >   * SOFTWARE.
> >   */
> >
> > -#pragma once
> > -
> >  #ifndef _VERBS_H_
> >  #define _VERBS_H_
> >
> > -#include <initguid.h>
> >  #include <iba/ib_ci.h>
> >
> >  static inline USHORT VerbsVersion(UINT8 Major, UINT8 Minor)
> > @@ -50,9 +47,6 @@ static inline UINT8 VerbsVersionMinor(US
> >  	return (UINT8) Version;
> >  }
> >
> > -DEFINE_GUID(GUID_RDMA_INTERFACE_VERBS, 0xf0ebae86, 0xedb5,
> 0x4b40,
> > -			0xa1, 0xa, 0x44, 0xd5, 0xdb, 0x3b, 0x96, 0x4e);
> > -
> >  typedef struct _RDMA_INTERFACE_VERBS
> >  {
> >  	INTERFACE		InterfaceHeader;
> > @@ -61,3 +55,6 @@ typedef struct _RDMA_INTERFACE_VERBS
> >  }	RDMA_INTERFACE_VERBS;
> >
> >  #endif // _VERBS_H_
> > +
> > +DEFINE_GUID(GUID_RDMA_INTERFACE_VERBS, 0xf0ebae86, 0xedb5,
> 0x4b40,
> > +			0xa1, 0xa, 0x44, 0xd5, 0xdb, 0x3b, 0x96, 0x4e);
> 
> The documentation referenced also adds #ifdef DEFINE_GUID around GUID
> definitions.

I'll fix this in in the next version of the patch.

> > Index: inc/kernel/iba/ib_cm_ifc.h
> >
> ===================================================================
> > --- inc/kernel/iba/ib_cm_ifc.h	(revision 3068)
> > +++ inc/kernel/iba/ib_cm_ifc.h	(working copy)
> > @@ -30,8 +30,6 @@
> >  #ifndef _ib_cm_ifc_h_
> >  #define _ib_cm_ifc_h_
> >
> > -#include <initguid.h>
> > -#include <iba/ib_al_ifc.h>
> >  #include <iba/ib_types.h>
> >  #include <iba/ib_al.h>
> >
> > @@ -304,10 +302,6 @@ static inline UINT8 IbaCmVersionMinor(US
> >  	return (UINT8) Version;
> >  }
> >
> > -// {6A11D060-8957-49e6-BE2A-01EDF1BD22B3}
> > -DEFINE_GUID(GUID_INFINIBAND_INTERFACE_CM, 0x6a11d060, 0x8957,
> 0x49e6,
> > -			0xbe, 0x2a, 0x1, 0xed, 0xf1, 0xbd, 0x22, 0xb3);
> > -
> >  typedef struct _INFINIBAND_INTERFACE_CM
> >  {
> >  	INTERFACE			InterfaceHeader;
> > @@ -315,4 +309,9 @@ typedef struct _INFINIBAND_INTERFACE_CM
> >
> >  }	INFINIBAND_INTERFACE_CM;
> >
> > -#endif // _ib_cm_ifc_h_
> > \ No newline at end of file
> > +#endif // _ib_cm_ifc_h_
> > +
> > +
> > +// {6A11D060-8957-49e6-BE2A-01EDF1BD22B3}
> > +DEFINE_GUID(GUID_INFINIBAND_INTERFACE_CM, 0x6a11d060, 0x8957,
> 0x49e6,
> > +			0xbe, 0x2a, 0x1, 0xed, 0xf1, 0xbd, 0x22, 0xb3);
> 
> See comment above for verbs.h

Same.

Thanks for the comments!
-Fab