[ofw] RE: [ofa-general] PATCH[1/6] Windows port of libibmad - mad.h

[Sean Hefty]

>What is the purpose of those patches? RFC? Inclusion to a main stream?

The goal is inclusion to the mainline, but giving a more concrete view of the
changes invovled.

>> diff -aur libibmad-1.2.2/include/infiniband/mad.h
>libibmad/include/infiniband/mad.h
>> --- libibmad-1.2.2/include/infiniband/mad.h	2008-08-31 07:15:05.000000000 -
>0700
>> +++ libibmad/include/infiniband/mad.h	2008-12-17 17:02:54.873046600
-0800
>> @@ -33,8 +33,10 @@
>>  #ifndef _MAD_H_
>>  #define _MAD_H_
>>
>> -#include <stdint.h>
>> -#include <string.h>
>> +/* use complib for portability */
>> +#include <complib/cl_types.h>
>> +#include <complib/cl_byteswap.h>
>> +#include <complib/cl_debug.h>
>
>Currently libibmad doesn't depend from complib. It would be really nice
>to not new dependencies (normally we build libibmad before complib,
>which is part of OpenSM).

The management stack does depend on complib, so I'm not sure that we buy much by
avoiding this dependency.  To avoid it, we need an alternate solution for
problems that complib is alreading solving.  (Arlin wrote these patches, so my
list may be off or incomplete.)

cl_types.h - provides the definitions for uint32_t and similar definitions
cl_byteswap.h - provides ntohll typdef
cl_debug - added cl_msg_out and CL_ASSERT defs

>I wrote in another email. It would be nice to minimize a number of
>needed changes and number of #ifdef introduced.

Use of complib is probably the best alternative to minimizing #ifdefs.
 
>If we will add "extern" keyword for exported symbols and somewhere in
>windows-specific header file it will be redefined as
>
>#define extern __declspec(dllexport)

I don't think we want to get into redefining keywords.

>> -void	_set_field(void *buf, int base_offs, ib_field_t *f, uint32_t
val);
>> +void _set_field(void *buf, int base_offs, ib_field_t *f, uint32_t val);
>
>What is the change here? Maybe whitespaces which were added/stripped by
>mailer, but I don't see this.

It looks like the whitespace was changed from a tab to a space, but you removed
these anyway.

>> +MAD_EXPORT uint32_t mad_get_field(void *buf, int base_offs, int field);
>> +MAD_EXPORT void mad_set_field(void *buf, int base_offs, int field, uint32_t
>val);
>
>Windows don't like "inline"?

The compiler doesn't allow it in the header file.

- Sean