[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Freeipmi-devel] Re: [llnl-devel] Sounds good ... I'll start adding the
From: |
Anand Babu |
Subject: |
[Freeipmi-devel] Re: [llnl-devel] Sounds good ... I'll start adding the packet dump functions into a new |
Date: |
Wed, 03 Dec 2003 05:08:45 -0800 |
User-agent: |
Gnus/5.1002 (Gnus v5.10.2) Emacs/21.3 (gnu/linux) |
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>
- [Freeipmi-devel] Re: [llnl-devel] Sounds good ... I'll start adding the packet dump functions into a new,
Anand Babu <=