Start Free Trial

asked on

improve padding addition code

I need an improvement to the getMemberListSize methods using shift opration etc...

/// This structure is defined for a little endian platform.
/// 
///     0               1               2               3
///     0 1 2 3 4 5 6 7 0 1 2 3 4 5 6 7 0 1 2 3 4 5 6 7 0 1 2 3 4 5 6 7
///    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
///    |    RadioId    |   RS  |   DS  |T|        ACN          |  RRRR |
///    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
///    |                        MMI                    |   memberId    |
///    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
///    |   |memberId...        |    memberId...    |    memberId...    |  
///    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
///    ...
 
/// </summary>
struct RadioEvStruct
{ 
   /////////////////////////////////////////////////////////////////////
   //        First Line of packet
   /////////////////////////////////////////////////////////////////////
   unsigned __int16 RadioId:8; 	            //0-255 possible patch Id
   unsigned __int16 RadioState:4;            //(1)Keyed,(0)Not Keyed
   unsigned __int16 DimState:4;              //(1)Active,(0)Deactive
   unsigned __int16 MMI:1;                   //(0)group,(1)group (tetra)
   unsigned __int16 MemberNb:11;             // The number of members 
                                             // up to 1024
   unsigned __int16 Reserved:4;  	            // Reserved bits for future use.
 
   /////////////////////////////////////////////////////////////////////
   //        The rest of the packet
   /////////////////////////////////////////////////////////////////////
   unsigned char*   aMemberList;         // Contains 0 or 1 MMI (24 bits)
                                         // + 0 to 50 members (10 bits each)                                        
                                         // members + (optional) required 
                                         // padding to avoid alignment problem.
 
   /// <summary>
   /// Comparison operator.
   /// </summary>
   //////////////////////////////////////////////////////////////////////////
   // memberwise comparison is safer because of the potential padding problem 
   // Here we assume there is no padding or alignment problems as well as   
   // everything is little indian. 
   // The length of the MemberList + 4 is multiple of 4.
 
   bool operator == (RadioEvStruct RadioEvStatus)
   {
      return  memcmp (this, & RadioEvStatus, getMemberListSize() + 4);  
   }
 
   /// <summary>
   /// Inverse Comparaison operator.
   /// </summary>
   bool operator != (RadioEvStruct RadioEvStatus)
   {
      return !(*this == RadioEvStatus);
   }
   
   unsigned __int16 getMemberListSize()
   {
   	// calculates and returns the size of the memberList based
	// on the values of MMI and MemberNb fields.
 
	   unsigned __int16 uGroupSize = this.MMI ? 3:2;
      unsigned __int16 uTotalBitsSize = (uGroupSize + this.MemberNb)*32;
      return (uTotalBitsSize + (( 4 - uTotalBitsSize % 4)%4))/8;
 
   }
};

Open in new window

ASKER

Sorry
a quick correction:

unsigned __int16 getMemberListSize()
{
     // calculates and returns the size of the memberList based
      // on the values of MMI and MemberNb fields.

       unsigned __int16 uGroupSize = this.MMI ? 3:2;
unsigned __int16 uTotalBitsSize = (uGroupSize + this.MemberNb)*32;
return (uTotalBitsSize + (( 32 - uTotalBitsSize % 32)%32))/8;

}

What do you mean by improve ?

You mean something like:

unsigned __int16 uGroupSize = MMI ? 3:2;
unsigned __int16 uTotalBitsSize = (uGroupSize + MemberNb) << 6;
return (uTotalBitsSize + (( 32 - (uTotalBitsSize & 31)) & 31)) >> 4;

sorry, forgot the this keyword... I don't have the complete implementation here :D, hope that helps to improve your function

ASKER

Sorry for not being clear enough,
By improve I mean inspect the code (verify that it works correctly) , make it more efficient
What phoenix did is almost what I need.
Just a question, does not hurt the fact that MemberNb is only 11 bits longs ?

uGroupSize + MemberNb would be something like (if MemberNb = 1024 set & uGroupSize = 3)

0000000000000011
XXXXX10000000000
-------------------------
XXXXX10000000011

Your result after by shifting by 6 or multiplying by 32 will be (1000000001100000), if you only want 11 bits you should mask the value by 0x0FFF.

ASKER

Yes I got it Thanks.
Last request I'm curious to know how the shift operation will be in this case :

unsigned __int16 uTotalBitsSize = (uGroupSize*8 + this.MemberNb*10);

>> By improve I mean inspect the code (verify that it works correctly)

What do you want the function to do ?

ASKER CERTIFIED SOLUTION

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

ASKER

The function will calculate the size of the list composed by 0 or 1 block of 24 bits and a number of blocks of 10 bits each then add the necessary padding to get the size multiple of 32 bits and returns the total size in bytes.

SOLUTION

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

ASKER

I've never asked to close the question, I already accepted the question and assigned points between two participants

ASKER

I've never asked to close the question, I already accepted the question and assigned points between two participants

ASKER

I think you have a bug in the system any time I try to accept the answer from multiple people

If you accept answers, then that automatically closes the question. I assume what you're referring to is the auto-close message, which appears if you select your OWN reply as the answer. If you select only expert replies as the answer, the question will be closed immediately without the auto-close message.