Converting 2 bytes to Short in C#

Question

I'm trying to convert two bytes into an unsigned short so I can retrieve the actual server port value. I'm basing it off from this protocol specification under Reply Format. I tried using BitConverter.ToUint16() for this, but the problem is, it doesn't seem to throw the expected value. See below for a sample implementation:

int bytesRead = 0;
while (bytesRead < ms.Length)
{
    int first = ms.ReadByte() & 0xFF;
    int second = ms.ReadByte() & 0xFF;
    int third = ms.ReadByte() & 0xFF;
    int fourth = ms.ReadByte() & 0xFF;
    int port1 = ms.ReadByte();
    int port2 = ms.ReadByte();
    int actualPort = BitConverter.ToUInt16(new byte[2] {(byte)port1 , (byte)port2 }, 0);
    string ip = String.Format("{0}.{1}.{2}.{3}:{4}-{5} = {6}", first, second, third, fourth, port1, port2, actualPort);
    Debug.WriteLine(ip);
    bytesRead += 6;
}

Given one sample data, let's say for the two byte values, I have 105 & 135, the expected port value after conversion should be 27015, but instead I get a value of 34665 using BitConverter.

Am I doing it the wrong way?

Solution 1

If you reverse the values in the BitConverter call, you should get the expected result:

int actualPort = BitConverter.ToUInt16(new byte[2] {(byte)port2 , (byte)port1 }, 0);

On a little-endian architecture, the low order byte needs to be second in the array. And as lasseespeholt points out in the comments, you would need to reverse the order on a big-endian architecture. That could be checked with the BitConverter.IsLittleEndian property. Or it might be a better solution overall to use IPAddress.HostToNetworkOrder (convert the value first and then call that method to put the bytes in the correct order regardless of the endianness).

Solution 2

BitConverter is doing the right thing, you just have low-byte and high-byte mixed up - you can verify using a bitshift manually:

byte port1 = 105;
byte port2 = 135;
ushort value = BitConverter.ToUInt16(new byte[2] { (byte)port1, (byte)port2 }, 0);
ushort value2 = (ushort)(port1 + (port2 << 8)); //same output

Solution 3

To work on both little and big endian architectures, you must do something like:

if (BitConverter.IsLittleEndian)
    actualPort = BitConverter.ToUInt16(new byte[2] {(byte)port2 , (byte)port1 }, 0);
else
    actualPort = BitConverter.ToUInt16(new byte[2] {(byte)port1 , (byte)port2 }, 0);

Comments

[Lasse Espeholt]

Are you sure this will work if the application is run on a different architecture?

[Mark Wilkins]

@lasseespeholt: That's a good point. It probably does need a check using IsLittleEndian. For a big-endian architecture, the order would need to be as given in the OP.

[Lasse Espeholt]

+1 for the "manual" approach. In my opinion BitConverter does way to many checks and an endian check which can break the code.

[shelbypereira]

from your solution: ushort value2 = (ushort)(port1 + (port2 << 8)); //same output this is the way to go, in most cases we need this type of conversion for a large volume of bytes and speed is likely to be an issue. I am currently using this in imaging applications. Overhead of creating the array and using bitconverter can be huge.

[shelbypereira]

solution from @BrokenGlass ushort value2 = (ushort)(port1 + (port2 << 8)); should be preferred in speed critical situations as bitconverter can and allocating the extra array can be slow.