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
_______________________________________________
ofw mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
