Is this an idiomatic C way to convert longs to a binary (char *) representation?

Question

The question is in the title I guess.

This is the temporary solution I came up with but I was wondering:

• If there are disadvantages to representing binary as char*. Is there a better way (considering i would want the ability of bit-shifting etc...)
• If there is obvious non-idiomatic C (or other errors) in the code below.

All suggestions welcome...

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

/* compile with 
    gcc -lm -std=c99 
*/

void binary_repr(unsigned long input) {
    int needed_digits = (int) (floor(log2(input)) + 1);
    char *ptr_binarray = malloc((needed_digits + 1) * sizeof (char));
    int idx = (needed_digits);

    if (ptr_binarray == NULL) {
         printf("Unable to allocate memory.");
     exit(1);
    } 
    else {
     do {
      idx--;
      if (input % 2 == 0) { 
       ptr_binarray[idx] = '0'; 
      } 
      else { 
       ptr_binarray[idx] = '1'; 
      }
      input = input / 2;

     } while (input > 0);

     ptr_binarray[needed_digits] = '\0';
     printf("%s\n", ptr_binarray);
     free(ptr_binarray);
     ptr_binarray = NULL;
    }
}

int main()
{
    binary_repr(8);
    binary_repr(14);
    binary_repr(4097);
    return 0;
}

Answer 1

You could use glibc to register a custom printf conversion specifier:

Customizing printf

Then you could do:

printf("Binary Representation:  %b\n", num);

This would be more flexible than having a call to printf() inside your function.

You would still need to specify a function to do the conversion; but then you could use (s)printf throughout the code.

Bastien Léonard's answer has a more idiomatic function to do the conversion, using bitwise-and instead of modulo 2, bit-shifting instead of division, and the ternary operator instead of if else.

Here is a related question:

is-there-a-printf-converter-to-print-in-binary-format

Answer 2

Looks approximately idiomatic to me, except that I'd write the loop something like:

char *writeptr = ptr_binarray + needed_digits;
*writeptr = 0;
do {
    --writeptr;
    *writeptr = (input % 2) + '0';
    input /= 2;
} while (input > 0);

No need for an integer index.

For this particular case, I wouldn't bother with malloc since you free in the same function. Just allocate a big enough char array on the stack:

char binarray[sizeof(unsigned long)*CHAR_BIT + 1];

Or make use of C99's variable length arrays:

char binarray[needed_digits + 1];

Also, if you're only using gcc, then rather than taking a logarithm you could consider using __builtin_clz to calculate needed_digits. That's not about idiomatic C, though, since it's the gcc dialect. But even without it, you don't need floating point math to figure out how many digits are needed:

http://graphics.stanford.edu/~seander/bithacks.html#IntegerLogObvious

Just noticed a probable error in that line, too - your do/while loop neatly handles the case where input is 0, but the first line doesn't since you can't take the log of 0.

Is there a better way (considering i would want the ability of bit-shifting etc...)

Not sure what you mean here. If you want to do operations like bit-shifting on a value, then don't convert it to a string like this. Keep it as a long int, and do your bit-shifting there.

Other minor things, since you're asking for general opinions. None of these are things I'd really criticise as long as you have a reason you've done them:

• Remove pointless parens around (needed_digits), it's just noise.
• Error message should probably go to stderr rather than stdout.
• I would always check the return value from malloc (or any other function that returns an error value) immediately, rather than have a line of code in between. So move the int idx = needed_digits line down to just before the 'do .. while' loop (since you're using std=c99. If it was c89, then you could still do that except that I'm going to recommend...).
• I wouldn't put an "else" after a conditional exit or return. But other people would do as you have, and the argument can probably get tribal.
• Personally I wouldn't multiply by sizeof(char) in malloc, since the size of the buffer allocated by malloc is measured in chars by definition. But others put it there so that every malloc consistently always has a sizeof, so again I can't claim my way is idiomatic. It's just better ;-)
• Clearing pointers after free is arguably worthwhile when they're in a structure, but not so much for automatics.

For each of the last three things, good C programming practice would not necessarily be to do as I do, but to agree a coding style with your colleagues / collaborators. The coding standard is allowed to be "do as you prefer", just so long as you agree not to argue, and not to "tidy up" each other's code.

Answer 3

itoa(value, output_buffer, base);

If you use 2 for base, you'll get the binary version in the string.

Note that I'm only answering "Is there a better way", not any other component of the question.

EDIT: Also, you may want to look at popular implementations of itoa to see how they did the multi-base conversion without requiring math functions (from -lm). I know a lot of itoa's I've seen are quite small and elegant and still quite powerful.

Answer 4

Are you converting to (char *) because you want the ability to bit shift? If so, are you aware of the bit shift operator?

short int n = 1;  //0x0001
n = n << 1;       //shift bits 1 place to the left
                  //n is now 2; 0x0010

Just for giggles, here's a routine for printing the binary representation using the bit shift operator:

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;
    }
}

Answer 5

There is no need to “convert” numbers to binary representation; they are already represented in binary inside memory. Using bitwise operators it's quite easy to play with binary representation:

#include <limits.h>
#include <stdio.h>

static void binary_repr(unsigned long input);

int main (void)
{
    binary_repr(0);
    binary_repr(1);
    binary_repr(16);

    return 0;
}

static void binary_repr(unsigned long input)
{
    unsigned int i;
    unsigned int nb_bits = sizeof(input) * CHAR_BIT;

    for (i = 0; i < nb_bits; ++i)
    {
        /* print the left-most bit */
        putchar((input & (1 << (nb_bits - 1))) == 0 ? '0' : '1');
        /* left-shift by onex */
        input <<= 1;
    }

    putchar('\n');
}

Answer 6

Yet another option. This simply loops over all bits from the most significant to the least significant and checks if they are set.

void binary_repr(unsigned long input)
{
    int i = sizeof(input) * 8 - 1;
    for (; i >= 0; --i) {
        putchar((input & (1 << i)) == 0 ? '0' : '1');
    }

    putchar('\n');
}

This doesn't do anything that hasn't already been suggested by other people here. It's just a method that's easier to remember for me.

Answer 7

Ok, another possible solution using look-up table:

#include <stdio.h>

#undef BIGENDIAN

#ifdef BIGENDIAN
enum { TSIZE = sizeof(int), INIT = 0, END = TSIZE };
#define op(x) ++(x)
#define cond(x) ((x) < END)

#else
enum { TSIZE = sizeof(int), INIT = TSIZE - 1, END = -1 };
#define op(x) --(x)
#define cond(x) ((x) > END)

#endif

static char *binstr[] = {
  "0000", // 0x0
  "0001", // 0x1
  "0010", // 0x2
  "0011", // 0x3
  "0100", // 0x4
  "0101", // 0x5
  "0110", // 0x6
  "0111", // 0x7
  "1000", // 0x8
  "1001", // 0x9
  "1010", // 0xA
  "1011", // 0xB
  "1100", // 0xC
  "1101", // 0xD
  "1110", // 0xE
  "1111", // 0xF
};


int main(void)
{
  int num, i;
  unsigned char *hex;

  hex = ((unsigned char *) &num);
  while(fscanf(stdin, "%i", &num) != EOF)
  {
    for(i = INIT; cond(i); op(i))
      printf("%s%s", binstr[hex[i]>>4], binstr[hex[i]&0xF]);
    printf("\n");
  }

  return 0;
}

PD: I only check with little-endian memory organization.

Answer 8

It's one of the great tragedies of Arabic notation that we put the most significant digit first. Almost all computations are easier when we start with the least significant digit:

void fprint_binary(FILE *fp, unsigned long n) {
  char digits[8*sizeof(n)+1];
  char *p = digits+sizeof(digits)-1;
  *p = '\0';
  unsigned long mask;
  for (mask = 1; mask; mask <<= 1)
    *--p = mask & n ? '1' : '0';
  while (*p == '0')
    p++;
  fprintf(fp, "%s", *p ? p : "0");
}

The comments about representation go double if you ever write code for a Turing machine (exercise for students, not practical).