Link to home
Start Free TrialLog in
Avatar of forums_mp
forums_mp

asked on

bit field manipulation / endian woes

There's the potential for a part 2 to this issue I'm faced with nonetheless, I've got code here that is littered with - and i quote ' compiler independent bit field maniuplation methods'.  The function GetBitsValueShifted is actually encapsulated in a class but for discussion purposes I've opted to provide a stripped down version of the source.  At issue:  Because of endian issues I'm not getting the desired result from the function GetBitsValueShifted.  
    Simply put for bits zero to 7, the value 0x113 should be 1 as opposed to 0x13.

How could I resolve  this?  (not sure if the solution lies in MMASKS)

# include <vector>
# include <ios>
# include <iomanip>
 
static const int MTOTAL_BITS_IN_TYPE=16;
const int MTOTAL_BITS_ZERO_BASED = MTOTAL_BITS_IN_TYPE-1;
const unsigned short MMASK_BIT0  = 0x0001;
const unsigned short MMASK_BIT1  = 0x0002;
const unsigned short MMASK_BIT2  = 0x0004;
const unsigned short MMASK_BIT3  = 0x0008;
const unsigned short MMASK_BIT4  = 0x0010;
const unsigned short MMASK_BIT5  = 0x0020;
const unsigned short MMASK_BIT6  = 0x0040;
const unsigned short MMASK_BIT7  = 0x0080;
const unsigned short MMASK_BIT8  = 0x0100;
const unsigned short MMASK_BIT9  = 0x0200;
const unsigned short MMASK_BIT10 = 0x0400;
const unsigned short MMASK_BIT11 = 0x0800;
const unsigned short MMASK_BIT12 = 0x1000;
const unsigned short MMASK_BIT13 = 0x2000;
const unsigned short MMASK_BIT14 = 0x4000;
const unsigned short MMASK_BIT15 = 0x8000;
const unsigned short MMASK_BIT16 = 0x0000;
 
//const unsigned short MMASK_BIT0  = 0x8000;
//const unsigned short MMASK_BIT1  = 0x4000;
//const unsigned short MMASK_BIT2  = 0x2000;
//const unsigned short MMASK_BIT3  = 0x1000;
//const unsigned short MMASK_BIT4  = 0x0800;
//const unsigned short MMASK_BIT5  = 0x0400;
//const unsigned short MMASK_BIT6  = 0x0200;
//const unsigned short MMASK_BIT7  = 0x0100;
//const unsigned short MMASK_BIT8  = 0x0080;
//const unsigned short MMASK_BIT9  = 0x0040;
//const unsigned short MMASK_BIT10 = 0x0020;
//const unsigned short MMASK_BIT11 = 0x0010;
//const unsigned short MMASK_BIT12 = 0x0008;
//const unsigned short MMASK_BIT13 = 0x0004;
//const unsigned short MMASK_BIT14 = 0x0002;
//const unsigned short MMASK_BIT15 = 0x0001;
//const unsigned short MMASK_BIT16 = 0x0000;
 
const unsigned short MMASKS[MTOTAL_BITS_IN_TYPE+1 ]= {
                                             MMASK_BIT0,
                                             MMASK_BIT1,
                                             MMASK_BIT2,
                                             MMASK_BIT3,
                                             MMASK_BIT4,
                                             MMASK_BIT5,
                                             MMASK_BIT6,
                                             MMASK_BIT7,
                                             MMASK_BIT8,
                                             MMASK_BIT9,
                                             MMASK_BIT10,
                                             MMASK_BIT11,
                                             MMASK_BIT12,
                                             MMASK_BIT13,
                                             MMASK_BIT14,
                                             MMASK_BIT15,
                                             MMASK_BIT16};
 
unsigned short GetBitsValueShifted(unsigned short mShortData_, 
                                   unsigned char startBit_, 
                                   unsigned char stopBit_)
{
   static unsigned int bitMask;
   static unsigned int numBits;
 
   unsigned short bitsValue = 0;
   
   // Create a bit mask at the bit location
   unsigned char stopBit = (stopBit_ + 15) % 15;
   unsigned char startBit = (startBit_ + 15) % 15;
   
   numBits = stopBit - startBit + 1;
   bitMask = (unsigned short) ((unsigned short) (MMASKS[numBits]-1));  //(2**numBits)-1
 
   bitsValue = ( (mShortData_ >> startBit) & bitMask);
 
   return (bitsValue);
}
 
typedef std::vector < unsigned short > USHORT_VEC ;
int main() 
{ 
  USHORT_VEC usv ( 30 ) ; 
  usv [ 2 ] = 0x113 ;
  for ( int pdx ( 0 ); pdx < usv.size(); ++pdx ) 
  {
    unsigned short mVal = GetBitsValueShifted  ( usv [ pdx ], 0, 7 ) ;
    std::cout << std::hex << "0x" << mVal << std::endl;
  }
  std::cin.get() ;
 
}

Open in new window

Avatar of Infinity08
Infinity08
Flag of Belgium image

>> Simply put for bits zero to 7, the value 0x113 should be 1 as opposed to 0x13.

bits 0 to 7 of the value 0x0113 are always 0x13, and endianness has nothing to do with that. Endianness is just how the value is stored, but it doesn't modify the value.
0x0113 & 0xFF is, in fact, 0x0013.
It is working correctly.
 
the code always bitshifts by 0 (meaning it shifts nothing) and then ands with a mask of 255. If you pass 275 (0x113) into this function you end up returning 275 & 255, which is 19 (0x13)

100010011 == 0x113 == 275
&11111111 ==  0xff == 255
---------------
         10011 == 0x13 == 19
Hmmm... it seems I take longer to read code that I8 and Dan (and I'm sure I8 is meant to me watching Rambo III) :)
>> (and I'm sure I8 is meant to me watching Rambo III) :)

Heh ... Toilet + EE break ;)
Avatar of forums_mp
forums_mp

ASKER

Something is amiss..

The MSB is bit 0 so when data arrives, data is 'deserialized' and gets stored in an array of unsigned short.  So given 0x113.   [0000][0001][0001][0011]

When decoded, I should end up with:

Bits 0.. 7 should produce a    1.  i.e: [0000][0001]
Bits 8..12 should produce a   2.  i.e. [0001][0]
Bits 13..15 should produce a 3   i.e. [011]

how would i modify function (GetBitsValueShifted) to get desired result.
  for ( int pdx ( 0 ); pdx < usv.size(); ++pdx ) 
  {
    unsigned short mPtype      = GetBitsValueShifted  ( usv [ pdx ], 0, 7 ) ;
    unsigned short mStationNum = GetBitsValueShifted  ( usv [ pdx ], 8, 12 ) ;
    unsigned short mPBay       = GetBitsValueShifted  ( usv [ pdx ], 13, 15 ) ;
    if  ( pdx == 2 ) 
    {
      std::cout << " Ptype =0x" << mPtype << " mStationNum=0x" << mStationNum 
                << " mPBay=0x" << mPBay 
                << std::endl; 
    }
  }

Open in new window

>> The MSB is bit 0

Bit 0 is the least significant bit (LSB), or the bit with the lowest value. The MSB is the bit with the highest value.

If you have the 16 bit value 0x0113, in binary, that is : 0000 0001 0001 0011. Bit 0 is 1, bit 1 is 1, bit 2 is 0, etc. (from right to left), up to bit 15, which is 0.

So :

bits 0 to 7 gives a value of 00010011 = 0x13 = 19
bits 8 to 12 gives a value of 00001 = 0x01 = 1
bits 13 to 15 gives a value of 000 = 0x00 = 0
Infinity; I understand that and its not what i want  My question is, what to do to get what I want  
SOLUTION
Avatar of DanRollins
DanRollins
Flag of United States of America image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Dan, I'd hope the solution lied within GetBitsValueShifted.  This way the underlying calls to GetBitsValueShifted remains unchanged (i.e 0..7 remains 0..7 as opposed to 8..15).  So given the source snippet. pType would be  1, mStationNum would be  2 and mPbay would equate to 3.


  for ( int pdx ( 0 ); pdx < usv.size(); ++pdx ) 
  {
    unsigned short mPtype      = GetBitsValueShifted  ( usv [ pdx ], 0, 7 ) ;
    unsigned short mStationNum = GetBitsValueShifted  ( usv [ pdx ], 8, 12 ) ;
    unsigned short mPBay       = GetBitsValueShifted  ( usv [ pdx ], 13, 15 ) ;
    if  ( pdx == 2 ) 
    {
      std::cout << " Ptype =0x" << mPtype << " mStationNum=0x" << mStationNum 
                << " mPBay=0x" << mPBay 
                << std::endl; 
    }
  }

Open in new window

It would be possible to redo the GetBitsValueShifted() fn, but whatever you do, you can't pass individual values into it... you need to group them.  As you showed in your own example,
Bits 8..12 should produce a   2.  i.e. [0001][0] <<< parts of two data elements

at least one of the fields spans two of the data elements.  And the loop you use in calling the function is sending individual data elements into the function.  It just can't work.
Also: What difference does it make if you call the bit field (for instance) "0-2" or "13-15" ??
It's the same two bits either way.
 
>> Infinity; I understand that and its not what i want  My question is, what to do to get what I want

Well, it seems you want to re-define what MSB and LSB mean ... My response was to not do that, but rather to re-visit your understanding of what they mean, and align yourself to the way everybody else understands them ;)

If you do change GetBitsValueShifted to suit your understanding, you'll confuse everybody else reading your code, with possibly disastrous consequences.
DanRollins
|| Also: What difference does it make if you call the bit field (for instance)
||  "0-2" or "13-15" ?? It's the same two bits either way.
For discussion purposes, I provided a very stripped down version of _one_ problem area.   In other words, part of what's lacking here is 'context'.  You're not seeing big picture and the huge impact that I may end up having to do long term (i inherited this mess and I'd like to fix it..)

infinity08:
|| If you do change GetBitsValueShifted to suit your understanding,
|| you'll confuse everybody else reading your code, with possibly disastrous consequences
I dont disagree with you.  Sometime today I'll put together a complete example to highlight what I'm faced with and get ideas/perhaps source on how to overhaul this mess.  In the iterim i've redefined MSB (so another user can continue his testing)

 
unsigned short GetBitsValueShifted(unsigned short mShortData_, 
                                   unsigned char startBit_, 
                                   unsigned char stopBit_)
{
   static unsigned int bitMask;
   static unsigned int numBits;
 
   unsigned short bitsValue = 0;
   
   // Create a bit mask at the bit location
   unsigned char stopBit  = 15 - startBit_ ;
   unsigned char startBit = 15 - stopBit_ ;   
   numBits = stopBit - startBit + 1;
   bitMask = (unsigned short) ((unsigned short) (MMASKS[numBits]-1));  //(2**numBits)-1
   bitsValue = ( (mShortData_ >> startBit) & bitMask);
   return (bitsValue);
}

Open in new window

>> (i inherited this mess and I'd like to fix it..)

Ah, now it starts making sense heh ;)
What you may actually need is a flexible way to extract a range of bits from an array of unsigned short values.... The extraction logic would know that only the bottom four bits of each array element were populated.
But then I am not seeing the big picture -- perhaps because I've only been given a tiny peephole through which to look.  Here's an idea: Describe your situation in more detail. Lots of detail. Remember that this is the first time that any of us has ever even looked at your problem. Tell us everything.
>> i inherited this mess and I'd like to fix it
I know that feeling only too well :(
|| Here's an idea: Describe your situation in more detail. Lots of detail.
|| Remember that this is the first time that any of us has ever even looked at your problem.
|| Tell us everything.

For starters:  
Rename all .txt files to .h (apparently EE doesn't accept .h and .cpp as attachments).  

Similarilily rename:
   bitfields_ushort.doc to bitfields_ushort.cpp
   main.doc to main.cpp
   base_msg.doc to base_msg.cpp

Lastly, rename InFile.doc to InFile.bin

Reference layout.doc for information on the R1 message description.

The source code compiles with Version 9 (MSVC.NET)

Noteworthy:

The file InFile.bin contains 64 bytes of incoming data.

-      The data is deserialized and stored in the array of unsigned short. To do this the Deserialize method in C_Msg_Base strips off the command word, stores the 30 data words in the array of unsigned short and ignores the status word.

-      To set or get information pertaining to specific proxies in a word, individual methods within C_Msg_R1 is invoked

In my view this is one horrible piece of code.. Alas, help needed:
1) [xxx] Refers to my rants based on my understand of the serializer source (it appears to be a pile of - to put it mildly - crap).  I'd like stream line this thing.
2) Streamline base_msg.  Given a multi-threaded environment I suspect I may need to stick with char* (std::string from what i understand is not thread safe).
3) When viewed from the context of the word document, it appears the definitions of MSB and LSB defined in r1_msg.cpp is incorrect.. To proceed with my test activities I quickly redefined MSB and LSB in the c_bitfields_ushort class class (thus far I've only changed - i.e redefined MSB and LSB  in - GetBitsValueShifted) .  Given there's a litany of messages redefining MSB and LSB is a significant change.  In the interim, I'll like to hear if my assessment is correct (i.e the layout in r1_msg.cpp is incorrect) and if there's a viable solution that doesn't require changing the proxies in all the messages.


msg-integration.zip
I arrived late at this party, but it seems everyone has zonked out, maybe Rambo III was too much for Infinity and evilrix. My quick overview of the code gives me the feeling that the original author might not have fully understood how to handle byteordering and serialization. It appears to be very complex for byteordering / serialization API. But again, I only spent 5 minutes looking it over. I can say, from writing the bytecode loader for Parrot/Perl6 runtime, I don't think it took me 1/2 that much code. So, I concur with your opinion that it is one horrible piece of code, but I'm not sure where to start. I surely don't have time or desire to dig into it myself, I would probably just take the original requirements and start over on this one.
ASKER CERTIFIED SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
mrjoltcola,
>> I would probably just take the original requirements and start over...
That's pretty much exactly what I did :-)
I've never been good at working on other people's code ... Why on EARTH did he do THAT?!?!?! :-).  The task itself is not complicated once you have the specs.
Dan you are the man on this one, I must admit, or you were just bored. :)

>> but it seems everyone has zonked out, maybe Rambo III was too much for Infinity and evilrix.

lol. Rather ... dozed off, but just got woken up again ;)

I don't have the time to re-work it like DanRollins did (hats off), but then I don't need to any more heh.

Just a small note about the mask table : it seems to be a bit off ... :)
>> >> but it seems everyone has zonked out, maybe Rambo III was too much for Infinity and evilrix.
There were already enough grade A experts working this thread... better suited to provide an answer than me :)
|| Dan you are the man on this one, I must admit,
||  or you were just bored. :)
Dan is the reason why folks like me keep our EE membership.  

Infinity08 and evilrix let me down on this one :).  I was shocked infinity wasn't  the first to fire off a 'try this' - with accompanying source code.
Dan, I'll need to stare at this some more later, nonethelesss it seems like you eliminated the use of the the serializer class.?
>> I was shocked infinity wasn't  the first to fire off a 'try this' - with accompanying source code.

;) My time management capabilities are being stretched to the limit at the moment ... sorry :)
>> it seems like you eliminated the use of the the serializer class.?
I eliminated all of everything but what you see there.  It provides the significnat functionality, but you will need to add back some of the other stuff (if you need it).
>> Infinity08 and evilrix let me down on this one
Like I said, there were already enough experts working on this.
|| It would actually be easier to just the real bit layout than the "backword" one that you have.  
|| You didn't want to do that, so I had to convert from the documentation notation into real values.
Question on this backward issue.  Why is it necessary to redefine MSB for the case where I'm tryin gto pull out individual bits yet that's not necessary for the case where I want to grab all 16 bits?  Seems odd to me.


SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Nice work Dan. :)
thanks :-)