Print an int in binary representation using C

Question

Hi All,

I'm looking for a function to allow me to print the binary representation of an int. What I have so far is;

char *int2bin(int a)
{
 char *str,*tmp;
 int cnt = 31;
 str = (char *) malloc(33); /*32 + 1 , because its a 32 bit bin number*/
 tmp = str;
 while ( cnt > -1 ){
      str[cnt]= '0';
      cnt --;
 }
 cnt = 31;
 while (a > 0){
       if (a%2==1){
           str[cnt] = '1';
        }
      cnt--;
        a = a/2 ;
 }
 return tmp;

}

But when I call

printf("a %s",int2bin(aMask)) // aMask = 0xFF000000

I get output like;

0000000000000000000000000000000000xtpYy (And a bunch of unknown characters.

Is it a flaw in the function or am I printing the address of the character array or something? Sorry, I just can't see where I'm going wrong.

Kind regards,

Gavin

NB The code is from here

EDIT: It's not homework FYI, I'm trying to debug someone else's image manipulation routines in an unfamiliar language. If however it's been tagged as homework because it's an elementary concept then fair play.

Answer 1

Your string isn't null-terminated. Make sure you add a '\0' character at the end of the string; or, you could allocate it with calloc instead of malloc, which will zero the memory that is returned to you.

By the way, there are other problems with this code:

• As used, it allocates memory when you call it, leaving the caller responsible for free()ing the allocated string. You'll leak memory if you just call it in a printf call.
• It makes two passes over the number, which is unnecessary. You can do everything in one loop.

Here's an alternative implementation you could use.

#include <stdlib.h>
#include <limits.h>

char *int2bin(unsigned n, char *buf)
{
    #define BITS (sizeof(n) * CHAR_BIT)

    static char static_buf[BITS + 1];
    int i;

    if (buf == NULL)
        buf = static_buf;

    for (i = BITS - 1; i >= 0; --i) {
        buf[i] = (n & 1) ? '1' : '0';
        n >>= 1;
    }

    buf[BITS] = '\0';
    return buf;

    #undef BITS
}

Usage:

printf("%s\n", int2bin(0xFF00000000, NULL));

The second parameter is a pointer to a buffer you want to store the result string in. If you don't have a buffer you can pass NULL and int2bin will write to a static buffer and return that to you. The advantage of this over the original implementation is that the caller doesn't have to worry about free()ing the string that gets returned.

A downside is that there's only one static buffer so subsequent calls will overwrite the results from previous calls. You couldn't save the results from multiple calls for later use. Also, it is not threadsafe, meaning if you call the function this way from different threads they could clobber each other's strings. If that's a possibility you'll need to pass in your own buffer instead of passing NULL, like so:

char str[33];
int2bin(0xDEADBEEF, str);
puts(str);

Answer 2

A couple of things:

int f = 32;
int i = 1;
do{
  str[--f] = i^a?'1':'0';
}while(i<<1);

• It's highly platform dependent, but maybe this idea above gets you started.
• Why not use memset(str, 0, 33) to set the whole char array to 0?
• Don't forget to free()!!! the char* array after your function call!

Answer 3

Two things:

1. Where do you put the NUL character? I can't see a place where '\0' is set.
2. Int is signed, and 0xFF000000 would be interpreted as a negative value. So while (a > 0) will be false immediately.

Aside: The malloc function inside is ugly. What about providing a buffer to int2bin?

Answer 4

Here's another option that is more optimized where you pass in your allocated buffer. Make sure it's the correct size.

// buffer must have length >= sizeof(int) + 1
// Write to the buffer backwards so that the binary representation
// is in the correct order i.e.  the LSB is on the far right
// instead of the far left of the printed string
char *int2bin(int a, char *buffer, int buf_size) {
    buffer += (buf_size - 1);

    for (int i = 31; i >= 0; i--) {
        *buffer-- = (a & 1) + '0';

        a >>= 1;
    }

    return buffer;
}

#define BUF_SIZE 33

int main() {
    char buffer[BUF_SIZE];
    buffer[BUF_SIZE - 1] = '\0';

    int2bin(0xFF000000, buffer, BUF_SIZE - 1);

    printf("a = %s", buffer);
}

Answer 5

Two simple versions coded here (reproduced with mild reformatting).

#include <stdio.h>

/* Print n as a binary number */
void printbitssimple(int n) 
{
    unsigned int i;
    i = 1<<(sizeof(n) * 8 - 1);

    while (i > 0) 
    {
        if (n & i)
            printf("1");
        else
            printf("0");
        i >>= 1;
    }
}

/* Print n as a binary number */
void printbits(int n) 
{
    unsigned int i, step;

    if (0 == n)  /* For simplicity's sake, I treat 0 as a special case*/
    {
        printf("0000");
        return;
    }

    i = 1<<(sizeof(n) * 8 - 1);

    step = -1; /* Only print the relevant digits */
    step >>= 4; /* In groups of 4 */
    while (step >= n) 
    {
        i >>= 4;
        step >>= 4;
    }

    /* At this point, i is the smallest power of two larger or equal to n */
    while (i > 0) 
    {
        if (n & i)
            printf("1");
        else
            printf("0");
        i >>= 1;
    }
}

int main(int argc, char *argv[]) 
{
    int i;
    for (i = 0; i < 32; ++i) 
    {
        printf("%d = ", i);
        //printbitssimple(i);
        printbits(i);
        printf("\n");
    }

    return 0;
}

Answer 6

A few suggestions:

• null-terminate your string
• don't use magic numbers
• check the return value of malloc()
• don't cast the return value of malloc()
• use binary operations instead of arithmetic ones as you're interested in the binary representation
• there's no need for looping twice

Here's the code:

#include <stdlib.h>
#include <limits.h>

char * int2bin(int i)
{
    size_t bits = sizeof(int) * CHAR_BIT;

    char * str = malloc(bits + 1);
    if(!str) return NULL;
    str[bits] = 0;

    // type punning because signed shift is implementation-defined
    unsigned u = *(unsigned *)&i;
    for(; bits--; u >>= 1)
     str[bits] = u & 1 ? '1' : '0';

    return str;
}