Hi David,

Thanks.
I see now that alignment change may bring trouble.
It makes more sense to keep it as it is for Unix systems than to change it.
Well, we can postpone the change until the next major version change.

As for integer types' sizes - they are more problematic.
Let me picture an example.

Marshalling packs simple types in the following way:

========For example for SaHpiInt64T===================
SaHpiInt64T x;
...
unsigned char output[];
...
memcpy( &output[pos], &x, sizeof(SaHpiInt64T) );
pos += sizeof(SaHpiInt64T);
======================================================

And demarshalling unpacks in the following way:

========For example for SaHpiInt64T===================
SaHpiInt64T x;
...
unsigned char input[];
...
memcpy( &x, &input[pos], sizeof(SaHpiInt64T) );
x = Swap_LE_BE_If_Needed( x );
pos += sizeof(SaHpiInt64T);
======================================================

So we already have trouble if basic types sizes are different for  
client/server system.
And changing it will bring another trouble.

BTW, there was a request to run openhpi on system where sizeof(int) != 4  
and
we made special configure option "--enable-non32bit-int".
Don't know how that story ended.

        Anton Pak

On Mon, 16 Jul 2012 00:35:23 +0400, David Mckinley  
<[email protected]> wrote:

> Hi Anton,
>
> I was mistaken when I said changing the alignment would make the  
> implementation incompatible with the HPI specification.  Quoting from  
> the specification:
>
> "It is recommended that these types be defined such that the data sizes  
> and alignment of each data type are as indicated. The only requirement  
> for source compatibility is that the types be defined to be able to  
> contain at least the required data (e.g., at least signed 8-bit values  
> must be contained in the data type defined as SaHpiInt8T, etc.)  
> Following the full recommendations for data size and alignment, however,  
> may promote more binary compatibility."
>
> So, changing the header would not make it incompatible with the  
> specification, but it would cut into the "binary compatibility" which is  
> promoted by following the recommendations.  It may be that 8-byte  
> alignment of 64-bit values is what happens by default on "most  
> platforms", but the last time I checked, the IA32 platform aligns 64-bit  
> values on 4-byte boundaries by default, and I would guess that this one  
> remains an important platform to consider.
>
> The problem that will likely arise is that people will have application  
> code that was compiled on IA32 with the SAF-supplied header file, or  
> previous versions of the OpenHPI supplied header file, and those  
> programs will be broken when linked with the newer version of the  
> OpenHPI library.  Recompiling the application with the new OpenHPI  
> header would fix the problem, but (a) it is rather unfriendly to require  
> that to keep a program working, especially when using such a mature  
> interface, and (b) the problems created by this misalignment are subtle  
> and difficult to debug.
>
> To summarize, I'd change my statement from saying that this change will  
> be incompatible with the specification, to saying that I am afraid it is  
> asking for trouble.  So, I suggest proceeding very cautiously.
>
> Regards,
>
> David
>
>> -----Original Message-----
>> From: Anton Pak [mailto:[email protected]]
>> Sent: Sunday, July 15, 2012 1:20 PM
>> To: [email protected]; David Mckinley
>> Subject: Re: [Openhpi-devel] Integer/float types in SaHpi.h
>>
>> Hello David,
>>
>> My agruments are:
>>
>> - HPI Spec leaves basic type definition to implementation.
>> - 64 bit type is already aligned to 8 byte boundary on most platforms.
>> - using gcc-specific __attribute__ is not good.
>> - client app and base library reside on the same system so using native
>> alignments makes sense.
>>
>>      Anton Pak
>>
>>
>> On Sun, 15 Jul 2012 18:31:25 +0400, David Mckinley
>> <[email protected]> wrote:
>>
>> > Anton,
>> >
>> > Removing the alignment setting would make the implementation
>> > incompatible with the HPI specification.  The data structures are
>> > passed in raw form between the user application program and the HPI
>> > library - which is the interface that SaHpi.h is actually specifying.
>> >
>> > Regards,
>> >
>> > David McKinley
>> >
>> >> -----Original Message-----
>> >> From: Anton Pak [mailto:[email protected]]
>> >> Sent: Saturday, July 14, 2012 5:32 PM
>> >> To: OpenHPI-devel
>> >> Subject: [Openhpi-devel] Integer/float types in SaHpi.h
>> >>
>> >> Hello!
>> >>
>> >> While looking into the question about 32/64-bit Client/Server mixing
>> >> from Mike Helles, I've realized that integer types defined in
>> SaHpi.h
>> >> header are not really cross-platform.
>> >>
>> >> Currently we have there:
>> >>
>> =====================================================================
>> >> ==
>> >> ==
>> >> /* unsigned 8-bit data, 1-byte alignment   */
>> >> typedef unsigned char SaHpiUint8T;
>> >>
>> >> /* unsigned 16-bit data, 2-byte alignment  */ typedef unsigned short
>> >> SaHpiUint16T;
>> >>
>> >> /* unsigned 32-bit data, 4-byte alignment  */ typedef unsigned int
>> >> SaHpiUint32T;
>> >>
>> >> /* unsigned 64-bit data, 8-byte alignment  */ typedef unsigned long
>> >> long int SaHpiUint64T __attribute__((__aligned__(8)));
>> >>
>> >> /* signed 8-bit data, 1-byte alignment     */
>> >> typedef signed char SaHpiInt8T;
>> >>
>> >> /* signed 16-bit data, 2-byte alignment    */
>> >> typedef signed short SaHpiInt16T;
>> >>
>> >> /* signed 32-bit data, 4-byte alignment    */
>> >> typedef signed int SaHpiInt32T;
>> >>
>> >> /* signed 64-bit data, 8-byte alignment    */
>> >> typedef signed long long int SaHpiInt64T
>> >> __attribute__((__aligned__(8)));
>> >>
>> >> /* 64-bit floating point, 8-byte alignment */ typedef double
>> >> SaHpiFloat64T __attribute__((__aligned__(8)));
>> >>
>> =====================================================================
>> >> ==
>> >> ==
>> >>
>> >> For most platforms char, short, int and long long give us 8, 16, 32
>> >> and
>> >> 64 bit types.
>> >> However it is not true for all platforms.
>> >>
>> >> There is standard C99 stdint.h header file that defines integer
>> types
>> >> of specified bit size.
>> >> Unfortunately  Microsoft C compiler is still not C99 compliant and
>> >> does not provide stdint.h.
>> >>
>> >> Also it is not clear why we need alignment for 64 bit types.
>> >> The data structures are always go through marshalling layer and
>> never
>> >> passed in raw form between client and server.
>> >>
>> >> My suggestions are:
>> >>
>> >> - Make use of int8_t, uint8_t, int16_t, uint16_t... types from C99
>> >> stdint.h.
>> >> - For Windows make of use of:
>> >> -- Way A) __int8t, unsigned __int8_t... types.
>> >> -- Way B) stdint.h replacements (some projects provide them, for
>> >> example
>> >> pstdint.h)
>> >> - Remove alignment setting
>> >>
>> >> What say?
>> >>
>> >>   Anton Pak
>> >>
>> >> --------------------------------------------------------------------
>> -
>> >> --
>> >> -------
>> >> Live Security Virtual Conference
>> >> Exclusive live event will cover all the ways today's security and
>> >> threat landscape has changed and how IT managers can respond.
>> >> Discussions will include endpoint security, mobile security and the
>> >> latest in malware threats.
>> >> http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
>> >> _______________________________________________
>> >> Openhpi-devel mailing list
>> >> [email protected]
>> >> https://lists.sourceforge.net/lists/listinfo/openhpi-devel
>> >
>> > ---------------------------------------------------------------------
>> -
>> > --------
>> > Live Security Virtual Conference
>> > Exclusive live event will cover all the ways today's security and
>> > threat landscape has changed and how IT managers can respond.
>> > Discussions will include endpoint security, mobile security and the
>> > latest in malware threats.
>> > http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
>> > _______________________________________________
>> > Openhpi-devel mailing list
>> > [email protected]
>> > https://lists.sourceforge.net/lists/listinfo/openhpi-devel

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Openhpi-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openhpi-devel

Reply via email to