7

I need to copy the data from 2 char (8 bit long) to a single short (16 bit long). I tried two different ways, but can't get it to work.

void char2short(char* pchar, short* pshort)
{
    memcpy(pshort    , pchar + 1    , 1);
    memcpy(pshort + 1, pchar    , 1);
}

And the other one:

void char2short(char* pchar, short* pshort)
{
   short aux;
   aux = ((*pchar & 0x00FF) << 8) | ((*(pchar+1) & 0xFF00) >> 8);
   *pshort = aux;
}
3
  • You can't use memcpy this way, because ist cannot cast these dataytpes automaticallay. You need to iterate over the src array and cast every value to short
    – Westranger
    Sep 11, 2014 at 12:25
  • 5
    You need to know if the short is represented as little-endian or big-endian.
    – interjay
    Sep 11, 2014 at 12:27
  • 1
    You need to know the endianness of pchar and pshort. If they are the same, use memcpy((void *)pshort, (const void*)pchar, sizeof(short));
    – Alex F
    Sep 11, 2014 at 12:38

4 Answers 4

9
#include <stdio.h>


void char2short(unsigned char* pchar, unsigned short* pshort)
{
  *pshort = (pchar[0] << 8) | pchar[1];
}

int main()
{
  unsigned char test[2];
  unsigned short result = 0;

  test[0] = 0xAB;
  test[1] = 0xCD;
  char2short(test, &result);
  printf("%#X\n",result);
  return 0;
}

this will do the job.

4
  • 3
    Endianess is not an issue here, at least when assuming the data under pchar does not change depending on the system's endianess, which is not the case in your example.
    – alk
    Sep 11, 2014 at 12:46
  • 2
    If this is a 16 bit system you'll get problems when left shifting a signed number 8 bits. That invokes undefined behavior. Don't use the sloppy default integer types when writing bit hacks!
    – Lundin
    Sep 11, 2014 at 13:57
  • 1
    Checking out here a few years later to tell you your answer help me get my thesis done and therefore my degree title! You rock!
    – ANIMATEK
    Feb 27, 2019 at 14:37
  • Gratulation. Good to hear that I could help you.
    – mch
    Feb 27, 2019 at 17:05
6

Assuming pchar is an array that contains your 2 chars, how about:

*pshort = (uint16_t)(((unsigned int)pchar[0]) |
                    (((unsigned int)pchar[1])<<8));

P.S. This work for little endianess.

1
  • 2
    Endianess is not an issue here, at least when assuming the data under pchar does not change depending on the system's endianess.
    – alk
    Sep 11, 2014 at 12:49
5

Others didn't explain why your code didn't work, so I'll take a quick stab at it:

memcpy(pshort    , pchar + 1    , 1);
memcpy(pshort + 1, pchar    , 1);

Adding to a pointer TYPE * p moves the pointer by increments of sizeof( TYPE ) (so it does point at the next element, remember this is only defined if inside an array). So while pchar + 1 is correct, pshort + 1 is not (as it's addressing the next short).

aux = ((*pchar & 0x00FF) << 8) | ((*(pchar+1) & 0xFF00) >> 8);

Errr.... the right hand side is broken in more ways than one. First, *(pchar+1) is a char, and & 0xFF00 on a char will always yield 0 (because a char is only 8 bits to begin with, at least on contemporary machines...). And then you shift that 8 bits to the right...?

And just in case you weren't aware of it, if you hadn't used 0x00FF on the left hand side (promoting *pchar to the width of the right-hand operand) but (char-sized) 0xFF, the result of that operation would still be of type char, and shifting that 8 bits to the left doesn't make much sense either (as the type doesn't get expanded magically).


Another way to go about this not mentioned yet is the union:

 #include <stdio.h>

 struct chars_t
 {
     // could also go for char[2] here,
     // whichever makes more sense semantically...
     char first;
     char second;
 };

 union combo_t
 {
      // elements of a union share the memory, i.e.
      // reside at the same address, not consecutive ones
      short shrt;
      struct chars_t chrs;
 };

 int main()
 {
     union combo_t x;
     x.chrs.first = 0x01;
     x.chrs.second = 0x02;
     printf( "%x", x.shrt );
     return 0;
  }

If you're using this in a larger context, beware of struct padding.

4
  • The union will make the code endianess-dependent. On a big endian system you will get the expected result 0x0102 but on little endian systems you will get 0x0201. And then indeed there's the padding hazard. So union is not a good idea, use bit shifts instead.
    – Lundin
    Sep 11, 2014 at 14:00
  • 1
    @Lundin: Depending on your needs, the endianess-dependent result might actually be what you're looking for. ;-) But I agree, union is somewhat dodgy. I included it for completeness' sake.
    – DevSolar
    Sep 11, 2014 at 14:04
  • Why would you intentionally want write endianess-dependent code? Endianess is a 100% bad thing no matter system.
    – Lundin
    Sep 11, 2014 at 14:05
  • 1
    @Lundin: I've come across systems once or twice where the existing APIs required me to write code that behaved differently depending on endianess. (Usually because the other party didn't bother to cater for endianess in the first place.) Sometimes changing existing APIs is not an option.
    – DevSolar
    Sep 11, 2014 at 14:31
3

When doing bitwise operations, use robust code with real fixed-size integers, of known signedness. This will prevent you from writing bugs related to implicit type conversions, causing unintended signedness. The char type is particularly dangerous, since it has implementation-defined signedness. It should never be used for storing numbers.

#include <stdint.h>

void char2short(const uint8_t* pchar, uint16_t* pshort)
{
  *pshort = ((uint16_t)pchar[0] << 8) | (uint16_t)pchar[1];
}

Your Answer

Reminder: Answers generated by Artificial Intelligence tools are not allowed on Stack Overflow. Learn more

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.