3

This is given:

signed short a, b;
a = -16;
b = 340;

Now I want to store these 2 signed shorts in one unsigned int and later retrieve these 2 signed shorts again. I tried this but the resulting shorts are not the same:

unsigned int c = a << 16 | b;

signed short ar, br;
ar = c >> 16;
br = c & 0xFFFF;
Teekeks
  • 196
  • 10
  • The standard way to do this is with type-punning through a union. See e.g.http://cellperformance.beyond3d.com/articles/2006/06/understanding-strict-aliasing.html – Erik Alapää Dec 09 '16 at 16:32
  • Did you check the size of short is half of that of int in your environment? – Sniper Dec 09 '16 at 16:44
  • I did check that it is 32bit and 16bit. – Teekeks Dec 09 '16 at 16:52
  • 1
    for portability I would suggest using the `uint16_t` and `uint32_t`, as the sizes of `int` and `short` are variable and may change. – Mgetz Dec 09 '16 at 16:58
  • "I tried this but the resulting shorts are not the same:" --> what were your results? – chux - Reinstate Monica Dec 09 '16 at 17:07
  • 1
    @Erik Alapää : Quite the opposite. The normal practice is to avoid any kind of type-punning unless you have a very very very good reason to use it. The OP's decision to use bitwise operations is the best. They just need to learn how to do it properly. – AnT stands with Russia Dec 09 '16 at 17:09
  • @AnT: Bitwise operations are of course very useful in low-level code, but look in good open source code - type-punning through unions is common, and often necessary to avoid aliasing problems. Otherwise, the code may have to be compiled with aliasing optimizations turned off. The standards are also more explicit about punning now: http://stackoverflow.com/questions/11639947/is-type-punning-through-a-union-unspecified-in-c99-and-has-it-become-specified – Erik Alapää Dec 10 '16 at 10:37

5 Answers5

2

OP almost had it right

#include <assert.h>
#include <limits.h>

unsigned ab_to_c(signed short a, signed short b) {
  assert(SHRT_MAX == 32767);
  assert(UINT_MAX == 4294967295);
  // unsigned int c = a << 16 | b; fails as `b` get sign extended before the `|`.
  // *1u insures the shift of `a` is done as `unsigned` to avoid UB 
  //    of shifting into the sign bit.
  unsigned c = (a*1u << 16) | (b & 0xFFFF);
  return c;
}

void c_to_ab(unsigned c, signed short *a, signed short *b) {
  *a = c >> 16;
  *b = c & 0xFFFF;
}
chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • @BLUEPIXY With `a << 16 | b`, `b` is converted to `int` as part of the usual integer promotions. Thus `(signed short) -1` becomes `(int)-1`. Then when or'ed with an `unsigned`, `(int)-1` becomes `(unsigned)0xFFFFFFFF` messing up the "or". The `b & 0xFFFF` prevents this. – chux - Reinstate Monica Dec 09 '16 at 17:11
1

Since a has a negative value,

unsigned int c = a << 16 | b;

results in undefined behavior.

From the C99 standard (emphasis mine):

6.5.7 Bitwise shift operators

4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits are filled with zeros. If E1 has an unsigned type, the value of the result is E1 x 2E2, reduced modulo one more than the maximum value representable in the result type. If E1 has a signed type and nonnegative value, and E1 x 2E2 is representable in the result type, then that is the resulting value; otherwise, the behavior is undefined.


You can explicitly cast the signed short to unsigned short to get a predictable behavior.

#include <stdio.h>

int main()
{
   signed short a, b;
   a = -16;
   b = 340;

   unsigned int c = (unsigned short)a << 16 | (unsigned short)b;

   signed short ar, br;
   ar = c >> 16;
   br = c & 0xFFFF;

   printf("ar: %hd, br: %hd\n", ar, br);
}

Output:

ar: -16, br: 340
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • 1
    Try `b = -16; a = 340;`, this code fails See [comment](http://stackoverflow.com/questions/41064702/store-2-signed-shorts-in-one-unsigned-int/41065221#comment69338098_41065221). Further `a = -16; unsigned int c = (signed)a << 16;` is UB for the reason in your citation. Code needs to do the shift as `unsigned`. – chux - Reinstate Monica Dec 09 '16 at 17:15
  • Meant that to be unsigned. – R Sahu Dec 09 '16 at 17:19
  • 1
    Even with `unsigned int c = (unsigned)a << 16 | b;`, `b = -16; a = 340;` fails. – chux - Reinstate Monica Dec 09 '16 at 17:20
  • @chux, both casting both `a` and `b` to `unsigned short` should work for all cases. – R Sahu Dec 09 '16 at 17:34
  • Disagree "casting both `a` and`b` to `unsigned short` should work". Casting `signed short a` via `(unsigned short) a` results in the now `unsigned short a` going through the usual integer promotions to `(int) a` (Given 16-bit `short`,32-bit `int`) and performs the shift as an `int`. Code still has UB per this answer's good citation. `signed short a` needs to be converted to `unsigned` before being shifted 16 to the left. The `(unsigned short)b` does help. – chux - Reinstate Monica Dec 09 '16 at 18:19
0

This is really weird, I've compiled your code and it works for me perhaps this is undefined behavior I'm not sure, however if I were you I'd add castings to explicitly avoid some bit loss that may or may not be caused by abusing two complement or compiler auto casting....

In my opinion what's happening is probably you shifting out all the bits in a... try this

unsigned int c = ((unsigned int) a) << 16 | b;
DrPrItay
  • 793
  • 6
  • 20
-1

This is because you are using an unsigned int, which is usually 32 bits and a negative signed short which is usually 16 bits. When you put a short with a negative value into an unsigned int, that "negative" bit is going to be interpreted as part of a positive number. And so you get a vastly different number in the unsigned int.

Storing two positive numbers would solve this problem....but you might need to store a negative one.

  • Well, I only want to use the unsigned int (aka the 32bit) for storage, the var c itself in the example does not have to make sense. but a should be equal to ar and b should be equal to br, which is not the case. – Teekeks Dec 09 '16 at 16:37
  • @BLIRPIXY no a and ar are the same type – Fredrik Dec 09 '16 at 16:49
-1

Not sure if this way of doing is good for portability or others but I use...

#ifndef STDIO_H
#define STDIO_H
#include <stdio.h>
#endif

#ifndef SDTINT_H
#define STDINT_H
#include <stdint.h>
#endif

#ifndef BOOLEAN_TE
#define BOOLEAN_TE
typedef enum {false, true} bool;
#endif  

#ifndef UINT32_WIDTH
#define UINT32_WIDTH 32 // defined in stdint.h, inttypes.h even in libc.h... undefined ??
#endif

typedef struct{
struct{                 // anonymous struct
    uint32_t    x;
    uint32_t    y;
};}ts_point;

typedef struct{
struct{                 // anonymous struct
    uint32_t    line;
    uint32_t    column;
};}ts_position;

bool    is_little_endian()
{
    uint8_t n = 1;
    return *(char *)&n == 1;
}

int main(void)
{
    uint32_t    x, y;
    uint64_t    packed;
    ts_point    *point;
    ts_position *position;

    x = -12;
    y = 3457254;
    printf("at start: x = %i | y = %i\n", x, y);
    if (is_little_endian()){
        packed = (uint64_t)y << UINT32_WIDTH | (uint64_t)x;
    }else{
        packed = (uint64_t)x << UINT32_WIDTH | (uint64_t)y;
    }
    printf("packed: position = %llu\n", packed);
    point = (ts_point*)&packed;
    printf("unpacked: x = %i | y = %i\n", point->x, point->y); // access via pointer
    position = (ts_position*)&packed;
    printf("unpacked: line = %i | column = %i\n", position->line, position->column);
    return 0;
}

I like the way I do as it's offer lots of readiness and can be applied in manay ways ie. 02x32, 04x16, 08x08, etc. I'm new at C so feel free to critic my code and way of doing... thanks

RoGKôT
  • 31
  • 6
  • Your `#ifndef` guards are attempting to **prevent** `stdio.h` and `stdlib.h` from being included, which is wildly non-portable, and also entirely nonsensical. – Willis Hershey Dec 08 '22 at 17:50