# Thread: Binary to decimal and reverse

1. When will the following loop ever end?
Code:
` while(binNum != NULL)`
After all binNum never changes in the loop.

Edit: As far as the output is concerned do you notice anything similar in the following two binary numbers?
10011
11001
Can you see the difference?

Jim

2. Well first off, in your while loop inside main, you never change the value of "binNum". It will never reach NULL.

3. And I'm pretty sure your logic for conversion is wrong. In your assignment on line 16, your adding '1' to a value that hasn't been initialized yet, which is essentially garbage. Same on line 19.

In your malloc call, it should look like:
Code:
`char* output = malloc(20*sizeof(output));`
This way, if you change the variable type of output, you don't have to change the data type in the sizeof operator.

I'm not sure why your trying to do the reverse of what your original question was? Seeing as how you probably haven't finished the first problem?

I'm trying to pass a decimal number to a function and convert it to binary and return it and print it out in main. But it prints out 1011 and then seg faults...not sure where it's tripping up

Code:
```int main(){
char* binNum = decToBin(25);
int i = 0;

while(binNum != NULL){
printf("%c", *(binNum+i));
i++;
}
}

char* decToBin(int dec){
char* output = malloc(20*sizeof(char));
int i = 0;
while(dec > 0){
if(dec % 2){
output[i] += '1';
}
else{
output[i] += '0';
}
dec /= 2;
i++;
}
return output;
}```

"main" needs to return 0

binNum needs to be freed to avoid a memory leak

binNum needs to be tested to see if decToBin failed
decToBin needs to check to see if malloc failed

output needs to be (8 * sizeof(dec)) +1 -> 8 bits per byte

output[i] = '1' or '0', it is not initialised, so += won't work

The string output needs to have a \0 put on the end of the string.

This is what you are probably after for that while loop in main
Code:
`while(*(binNum+i) != '\0')`

And after all that, your string is back the front (25 -> 0b10011) because the left side (msb) was calculated from the original number mod 2, where as the lsb of that number is the original number mod 2

Enjoy

5. malloc does not initialise the memory it gives you to all zeros. You then proceed to add values to each char.
You can either use calloc to obtain a zero-initialised block, or simply set the contents instead of adding to what is there.

6. Originally Posted by Click_here

"main" needs to return 0

binNum needs to be freed to avoid a memory leak

binNum needs to be tested to see if decToBin failed
decToBin needs to check to see if malloc failed

output needs to be (8 * sizeof(dec)) +1 -> 8 bits per byte

output[i] = '1' or '0', it is not initialised, so += won't work

The string output needs to have a \0 put on the end of the string.

This is what you are probably after for that while loop in main
Code:
`while(*(binNum+i) != '\0')`

And after all that, your string is back the front (25 -> 0b10011) because the left side (msb) was calculated from the original number mod 2, where as the lsb of that number is the original number mod 2

Enjoy

Thanks a bunch for the help everyone. I'm still getting 10011, and if I simply swap the 1 with 0 and 0 with 1 in my if else loop, it prints out 01100...which makes absolutely no sense

Code:
```int main(){
char* binNum = decToBin(25);
int i = 0;
while(*(binNum+i) != '\0'){
printf("%c", *(binNum+i));
i++;
}
free(binNum);
return 0;
}
char* decToBin(int dec){
char* output = malloc(8*sizeof(dec));
int i = 0;
while(dec > 0){
if(dec % 2){
output[i] = '1';
}
else{
output[i] = '0';
}
dec /= 2;
i++;
}
output[i] = '\0';
return output;
}```

7. Click_here wasn't saying your 1's and 0's were swapped, he was saying your string is backwards. That is, instead of printing
10011
you should print
11001
Start at the back and work your way to the front when you print.

8. Ahhhhh i see. Yeah I didn't quite understand that last part, thanks!

9. In your malloc call, your allocating enough space for eight(8) ints, but you're using a character pointer. This could get messy in the future. I would pick one, int or char, and stick with it.

10. Originally Posted by jwroblewski44
In your malloc call, your allocating enough space for eight(8) ints, but you're using a character pointer. This could get messy in the future. I would pick one, int or char, and stick with it.
That's okay, he needs a string to hold the binary representation of an int, which is enough to hold all the bits used to represent an int, which is sizeof(int) * bits_per_byte. That ensures that he has the right amount of space, even if an int is 2 bytes on an old system, 4 bytes (like a typical 32-bit system), 8 bytes (64-bit), etc. Typically bits_per_byte is 8, but it's not guaranteed. The standard actually provides a macro, CHAR_BIT, to use for this purpose.

@johngoodman:
Your malloc does not leave space for the terminating null character, which is a problem if you need all 32 bits. Nor does it account for systems which have more or less than 8 bits per byte (I've never seen/heard of such system, but they theoretically can exist). I would suggest this call:
Code:
`char* output = malloc(CHAR_BIT*sizeof(dec) + 1);`
CHAR_BIT will always be the right number of bits per byte, and the +1 ensures room for the null terminator required by all strings.

One more thing, what happens if you pass a negative number to decToBin? Your loop will never execute, and you'll have an empty string, despite the fact that even negative numbers have a binary representation. I suggest using unsigned int, like so:
Code:
`char* decToBin(unsigned int dec){`

11. CHAR_BIT is a good idea -> That is in the standard header <limits.h>

So all you hard work isn't wasted, make a function which rotates a string -> It's very easy to do.

Code:
```/*
* Function - rotateString
* inputs - (char *) input
* output - Returns (int)1 for success, (int)0 for fail
* behaviour - The string input points to is reversed
*                 i.e. "1234" -> "4321"
*/
int rotateString(char *input);

/******************************/

/* if s points to a string "10011" */
if (rotateString (s) != 1)
{
/*
* Rotate Failed
* s still points to "10011"
*/
}
else
{
/*
* Rotate Succeeded
* s now points to "11001"
*/
}```