Printing the bits of an integer using bitfields and union

Question

I am given a short int. I need to print the bits of it into two bytes. My code is as follows:

#include <stdio.h>

union info{
    short z;
    struct data{
        unsigned a:1;
        unsigned b:1;
        unsigned c:1;
        unsigned d:1;
        unsigned e:1;
        unsigned f:1;
        unsigned g:1;
        unsigned h:1;
  };
}t;

union byte{
short n;
struct inside{
    char p:8;
    char q:8;
  };
}v;

int main(void)
{
 short x;

 scanf("%hd",&x);

 v.n=x;

 t.z=v.q;

 printf("%d %d %d %d %d %d %d %d\n",t.h,t.g,t.f,t.e,t.d,t.c,t.b,t.a);

 t.z=v.p;

 printf("%d %d %d %d %d %d %d %d",t.h,t.g,t.f,t.e,t.d,t.c,t.b,t.a);

 return 0;
}

My code gives the correct result. But I think that I've written an unnecessarily long code. Moreover, for larger bytes ( like in case of int ) , the process seems to be tiresome. Can I write the code in a simpler way ?

Answer 1

Avoid global variables - there's no need for t and v to exist outside main().

Always check the return value of scanf() before using the written values.

Don't assume that CHAR_BIT is 8, or that sizeof (short) is 2. Neither of those is portable.

Don't assume a particular ordering of bit fields within a struct - that's entirely compiler-dependent.

Portable code needs a loop to print the bits, like this:

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

int main(void)
{
    short x;
    if (scanf("%hi", &x) != 1) {
        fputs("Input error\n", stderr);
        return EXIT_FAILURE;
    }

    unsigned short v = (unsigned short)x;

    unsigned short mask = -1u;                /* 11111... */
    mask -= (unsigned short)(mask / 2);       /* 10000... */

    while (mask) {
        printf("%d ", (v & mask) != 0);
        mask >>= 1;
    }
    printf("\n");
}

Answer 2

In addition to @Toby Speight's answer.

1. There are only four types allowed for a bit field. There are the follows: signed int, unsigned int, int, and _Bool. So using char in this case:

   struct inside{
       char p:8;
       char q:8;
   };
   

   is implementation-defined.

2. Use fixed integer types. In your code you use short the width of which is not well defined. If you want an object with a specific width, you should consider using fixed integer types from <stdint.h>, such as uint8_t or uint16_t.

3. Use an anonymous struct inside the union. Your code actually is not valid, because a is not a member of t as well as p is not a member of v. a is member of the inner data struct:

   union info{
        short z;
        struct data{
               unsigned a:1;
               unsigned b:1;
               unsigned c:1;
               unsigned d:1;
               unsigned e:1;
               unsigned f:1;
               unsigned g:1;
               unsigned h:1;
        };
   }t;
   

   You have to use anonymous structures in order to compile your code.

4. Bit fields are very implementation defined. You actually couldn't print bits of integer in a portable way because bit fields are very implementation defined. You have to care about a bit field width, about padding inside the structure, about allocation units inside the structure, about width of the integer, about byte ordering, and so on.

Answer 3

I agree with most of the suggestions from @TobySpeight, except for the loop variable. Consider:

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

int main(void) {
    short x;
    if (scanf("%hi", &x) != 1) {
        perror("Input error");
        return EXIT_FAILURE;
    }

    unsigned short v = (unsigned short)x;

    for (int i = 8*sizeof(v)-1; i >= 0; i--)
        printf("%u ", 1&(v>>i));
    putchar('\n');
    return 0;
}

You can have a simple integer loop variable. The way this works:

• i starts at 15, and decreases to 0
• for every digit, shift the number right by i, so that the digit in question is in the least-significant position
• Do a binary-and with 1, and then print the result.