c; converting 2 bytes to short and vice versa

Question

I want to convert array of bytes bytes1 (little endian), 2 by 2, into an array of short integers, and vice versa . I expect to get final array bytes2, equal to initial array bytes1. I have code like this:

  int i = 0;
  int j = 0;

  char *bytes1;
  char *bytes2;
  short *short_ints;

  bytes1 = (char *) malloc( 2048 );
  bytes2 = (char *) malloc( 2048 );
  short_ints = (short *) malloc( 2048 );

  for ( i=0; i<2048; i+=2)
   {
     short_ints[j] = bytes1[i+1] << 8 | bytes1[i] ;
     j++;
   }

  j = 0;

  for ( i=0; i<2048; i+=2)
     {
        bytes2[i+1] = (short_ints[j] >> 8)  & 0xff;
        bytes2[i] = (short_ints[j]) ;
        j++;
     }
  j = 0;

Now, can someone tell me why I haven't got bytes2 array, completely the same as bytes1 ? And how to do this properly?

Answer 1

Here's a program that demonstrates that you are experiencing the problem associated with bit-shifting signed integral values.

#include <stdio.h>
#include <stdlib.h>

void testCore(char bytes1[],
              char bytes2[],
              short short_ints[],
              int size)
{
   int i = 0;
   int j = 0;

   for ( i=0; i<size; i+=2)
   {
      short_ints[j] = bytes1[i+1] << 8 | bytes1[i] ;
      j++;
   }

   j = 0;

   for ( i=0; i<size; i+=2)
   {
      bytes2[i+1] = (short_ints[j] >> 8)  & 0xff;
      bytes2[i] = (short_ints[j]) ;
      j++;
   }

   for ( i=0; i<size; ++i)
   {
      if ( bytes1[i] != bytes2[i] )
      {
         printf("%d-th element is not equal\n", i);
      }
   }
}

void test1()
{
   char bytes1[4] = {-10, 0, 0, 0};
   char bytes2[4];
   short short_ints[2];
   testCore(bytes1, bytes2, short_ints, 4);
}

void test2()
{
   char bytes1[4] = {10, 0, 0, 0};
   char bytes2[4];
   short short_ints[2];
   testCore(bytes1, bytes2, short_ints, 4);
}

int main()
{
   printf("Calling test1 ...\n");
   test1();
   printf("Done\n");
   printf("Calling test2 ...\n");
   test2();
   printf("Done\n");
   return 0;
}

Output of the program:

Calling test1 ...
1-th element is not equal
Done
Calling test2 ...
Done

Udate

Here's a version of testCore that works for me:

void testCore(char bytes1[],
              char bytes2[],
              short short_ints[],
              int size)
{
   int i = 0;
   int j = 0;
   unsigned char c1;
   unsigned char c2;
   unsigned short s;

   for ( i=0; i<size; i+=2)
   {
      c1 = bytes1[i];
      c2 = bytes1[i+1];
      short_ints[j] = (c2 << 8) | c1;
      j++;
   }

   j = 0;

   for ( i=0; i<size; i+=2)
   {
      s = short_ints[j];
      s = s >> 8;
      bytes2[i+1] = s;
      bytes2[i] = short_ints[j] & 0xff;
      j++;
   }

   for ( i=0; i<size; ++i)
   {
      if ( bytes1[i] != bytes2[i] )
      {
         printf("%d-th element is not equal\n", i);
      }
   }
}

It is tested with:

char bytes1[4] = {-10, 0, 25, -4};

and

char bytes1[4] = {10, -2, 25, 4};

Answer 2

Suggest 2 functions. Do all combining and extraction as unsigned to remove issues with the sign bit in short and maybe char.

The sign bit is OP's code biggest problem. short_ints[j] = bytes1[i+1] << 8 | bytes1[i] ; likely does a sign extend with bytes1[i] conversion to int.
Also (short_ints[j] >> 8) does a sign extend.

// Combine every 2 char (little endian) into 1 short
void charpair_short(short *dest, const char *src, size_t n) {
  const unsigned char *usrc = (const unsigned char *) src;
  unsigned short *udest = (unsigned short *) dest;
  if (n % 2) Handle_OddError();
  n /= 2;
  while (n-- > 0) {
    *udest = *usrc++;
    *udest += *usrc++ * 256u;
    udest++;
  }
}

// Break every short into 2  char (little endian)
void short_charpair(char *dest, const short *src, size_t n) {
  const unsigned short *usrc = (const unsigned short *) src;
  unsigned char *udest = (unsigned char *) dest;
  if (n % 2) Handle_OddError();
  n /= 2;
  while (n-- > 0) {
    *udest++ = (unsigned char) (*usrc);
    *udest++ = (unsigned char) (*usrc / 256u);
    usrc++;
  }
}

int main(void) {
  size_t n = 2048;  // size_t rather than int has advantages for array index

  // Suggest code style: type *var = malloc(sizeof(*var) * N);
  // No casting of return
  // Use sizeof() with target pointer name rather than target type.
  char *bytes1 = malloc(sizeof * bytes1 * n);
  Initialize(bytes, n); //TBD code for OP-best to not work w/uninitialized data

  // short_ints = (short *) malloc( 2048 );
  // This is weak as `sizeof(short)!=2` is possible

  short *short_ints = malloc(sizeof * short_ints * n/2);
  charpair_short(short_ints, bytes1, n);

  char *bytes2 = malloc(sizeof * bytes2 * n);
  short_charpair(bytes2, short_ints, n);

  compare(bytes1, bytes2, n); // TBD code for OP

  // epilogue 
  free(bytes1);
  free(short_ints);
  free(bytes2);
  return 0;
}

Avoided the union approach as that is platform endian dependent.

Answer 3

Well, what you need is a UNION:

#include <stdio.h>
#include <string.h>

union MyShort {
    short short_value;
    struct {
        char byte1;
        char byte2;
    };
};

int main(int argc, const char * argv[])
{
    char a[4]="abcd";
    char b[4]="1234";
    short c[5]; c[4]=0;
    union MyShort d;
    for (int i = 0; i<4; i++) {
        d.byte1 = a[i];
        d.byte2 = b[i];
        c[i] = d.short_value;
    }//next i
    printf("%s\n", (char*)c);
    return 0;
}

the result should be a1b2c3d4.