[Freeipmi-devel] Re: [llnl-devel] Sounds good ... I'll start adding the packet dump functions into a new

[Anand Babu]

Let us restart the debate:

"struct / #pragma pack(1)" vs "byte array"
==========================================

"struct / #pragma pack(1)"
--------------------------
 - Elegant

"byte array"
------------
 - Most Portable - across compilers, platforms

Nobody likes the "byte array" model. Code is utterly unreadable. 
But when portability is #1 goal, there is no choice.

I will support "struct / #pragma pack" approach, until someone
convinces me with clear examples when and where this model will break,
how often we will hit them.

-ab

Ben Woodard <address@hidden> writes:

>Say I had a structure like:
>
>struct foo{
>       int type;
>       char status;
>       unsigned int code;
>       char name[5];
>       char reg_a:1;
>       char reg_b:3;
>};
>
>#define PKTFOO_NAME_OFF 0
>#define PKTFOO_TYPE_OFF 5
>#define PKTFOO_CODE_OFF 7
>#define PKTFOO_STATUS_OFF 9
>#define PKTFOO_REGISTER_OFF 10
>#define PKTFOO_A_BITOFF 1
>#define PKTFOO_B_BITOFF 2
>
>inline void marshal_string( char *buf, unsigned int offset, const char
>*str, unsigned int len){
>       memcpy(buf+offset,str,len);
>}
>
>inline void marchal_2byte_int_be( char *buf, unsigned int offset, int
>val){
>       int num=val&0xff00;
>       buf[offset]=num;
>       num=val&0xff;
>       buf[offset+1]=num;
>}
>
>...
>
>On Sat, 2003-11-15 at 10:07, Albert Chu wrote:
>> ipmi_error.c file at some point.
>> 
>> > We should take packing effect seriously and fix them, before the code
>> > evolves further.
>> >
>> > I'm not having a clear picture of how you are going to implement
>> > marshalling. Can you throw me an example code.
>> 
>> Ben Woodard has far more experience in this area than me, b/c he worked
>> on porting/fixing lpd for a number of years.  But he suggests something
>> along the lines of:
>> 
>> marshall_ipmi_pkt(ipmi_pkt *pkt, char *buf, int buflen) {
>>    memcpy(&buf[0], pkt->char_val, 1);
>>    memcpy(&buf[1], to_little_endian(pkt->int_val), 4);
>>    memcpy(&buf[5], pkt->netfn << 2 | pkt->lun, 1);
>> }
>> 
>> Unmarshalling would be the opposite, putting values into the appropriate
>> fields.  Its painful and mind numbing to code, but several developers
>> agree that it must be done to avoid problems down the road.
>> 
>> I found yet another reason to do the marshalling and unmarshalling.
>> 
>> struct foo {
>> RMCP bug??
>> X-Accept-Language: en
>> Content-Type: text/plain; charset=us-ascii
>> Content-Disposition: inline
>> Content-Transfer-Encoding: 7bit
>> 
>> Hey AB,
>> 
>> As far as I can tell, you are never converting the RMCP IANA Enterprise
>> number from little endian to big endian, and RMCP packets are required
>> to be in big endian form (12.3.1)...  Is it working??  Perhaps Intel
>> made their chips handle both??
>> 
>> --
>> Albert Chu
>> address@hidden
>> Lawrence Livermore National Laboratories
>> 
>> ----- Original Message -----
>> From: Albert Chu <address@hidden>
>> Date: Saturday, November 15, 2003 9:43 am
>> Subject: [Freeipmi-devel] Re: [llnl-devel] FreeIPMI conflict/issue list
>> 
>> > > Albert Chu <address@hidden> writes:
>> > > I think we should remove ipmi_errno and rename the file to
>> > > ipmi_error.c
>> > > 
>> > > You tell me what you are going to fix. I wont touch that part of the
>> > > code. :)
>> > 
>> > Sounds good ...  I'll start adding the packet dump functions into a 
>> > newipmi_error.c file at some point.
>> > 
>> > > We should take packing effect seriously and fix them, before the 
>> > code> evolves further.
>> > > 
>> > > I'm not having a clear picture of how you are going to implement
>> > > marshalling. Can you throw me an example code.
>> > 
>> > Ben Woodard has far more experience in this area than me, b/c he 
>> > workedon porting/fixing lpd for a number of years.  But he suggests 
>> > somethingalong the lines of:
>> > 
>> > marshall_ipmi_pkt(ipmi_pkt *pkt, char *buf, int buflen) {
>> >   memcpy(&buf[0], pkt->char_val, 1);
>> >   memcpy(&buf[1], to_little_endian(pkt->int_val), 4);
>> >   memcpy(&buf[5], pkt->netfn << 2 | pkt->lun, 1);
>> > }
>> > 
>> > Unmarshalling would be the opposite, putting values into the 
>> > appropriatefields.  Its painful and mind numbing to code, but 
>> > several developers
>> > agree that it must be done to avoid problems down the road.
>> > 
>> > I found yet another reason to do the marshalling and unmarshalling.
>> > 
>> > struct foo {
>> >   int a: 2;
>> >   int b: 6;
>> > };
>> > 
>> > On some compilers/machines/whatever foo's will be aligned "a:b" in
>> > memory, while others align it "b:a" ... 
>> > 
>> > Al
>> > 
>> > --
>> > Albert Chu
>> > address@hidden
>> > Lawrence Livermore National Laboratories
>> > 
>> > ----- Original Message -----
>> > From: Anand Babu <address@hidden>
>> > Date: Friday, November 14, 2003 4:05 pm
>> > Subject: Re: [llnl-devel] FreeIPMI conflict/issue list
>> > 
>> > > Albert Chu <address@hidden> writes:
>> > > >I didn't even notice the ipmi_errno variable.  Is this even 
>> > > necessary? 
>> > > >I would think this is only necessary if we wrote much higher level
>> > > >functions, such as "ipmi_setup_session" or something.  In the 
>> > current> >framework errors will exist in packet completion codes or in
>> > > >sendto/recvfrom.  I feel its ok for errors from sendto/recvfrom 
>> > to be
>> > > >passed back to the user via libc errno.  I still don't know your 
>> > > code as
>> > > >well as you, so you perhaps see something I don't see.
>> > > 
>> > > I think we should remove ipmi_errno and rename the file to
>> > > ipmi_error.c
>> > > 
>> > > You tell me what you are going to fix. I wont touch that part of the
>> > > code. :)
>> > > 
>> > > >Although its painful, and a lot of code, ultimately I think this 
>> > > is the
>> > > >safest approach.  The compiler aligns the structures in the 
>> > memory 
>> > > just>like we want them to, but there is no guarantee they'll do 
>> > > that in the
>> > > >future.  
>> > > >
>> > > >An added bonus is that marshall/unmarshall functions will allow 
>> > us to
>> > > >fix endian issues.  We have a lot of IBM laptops floating 
>> > around.  If
>> > > >any of the sysadmins want to use ipmipower on those laptops, 
>> > > libfreeipmi>will completly bomb right now.
>> > > 
>> > > We should take packing effect seriously and fix them, before the 
>> > code> evolves further.
>> > > 
>> > > I'm not having a clear picture of how you are going to implement
>> > > marshalling. Can you throw me an example code.
>> > > 
>> > > 
>> > > Happy Hacking
>> > > -- 
>> > > Anand Babu
>> > > CaliforniaDigital.com
>> > > Office# +1-510-687-7045
>> > > Cell# +1-510-396-0717
>> > > Home# +1-510-894-0586
>> > > 
>> > > Free as in Freedom <www.gnu.org>
>> > > 
>> > 
>> > 
>> > 
>> > _______________________________________________
>> > Freeipmi-devel mailing list
>> > address@hidden
>> > http://mail.nongnu.org/mailman/listinfo/freeipmi-devel
>> > 
>> 
>> 
>> _______________________________________________
>> llnl-devel mailing list
>> address@hidden
>> http://californiadigital.com/cgi-bin/mailman/listinfo/llnl-devel
>-- 
>Ben Woodard <address@hidden>
>Red Hat Inc.
>
>
>_______________________________________________
>llnl-devel mailing list
>address@hidden
>http://californiadigital.com/cgi-bin/mailman/listinfo/llnl-devel

-- 
Anand Babu
CaliforniaDigital.com
Office# +1-510-687-7045
Cell# +1-510-396-0717
Home# +1-510-894-0586

Free as in Freedom <www.gnu.org>