How do I save values to a pointer I have allocated memory to?

Question

I have made the following code:

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

char * convertToBinary(int baseTenNumber)
{
    int counter = 0;    
    char *result = NULL;
    int remainder = baseTenNumber;
    while(remainder)
    {
        result = (char *) realloc(result, sizeof(char) + 2);
        printf("%d\n", baseTenNumber);
        result[counter] = baseTenNumber%2;
        remainder/=2;
        counter++;
    }

    return result;
}

int main (void)
{
    int input = -1;

    while(input)
    {
        printf("Enter an integer: ");
        scanf("%d", &input);
        char * result = convertToBinary(input);
        printf("The binary version of %d is %s\n", input, result);

        memset(result, '0', sizeof(result));
        free(result);
    }


    return 0;
}

Yes, I know the binaries will be written backwards, I will get to that later. But why doesn't this program save my values?

I am trying to allocate memory on each iteration of the loop. I am not interested in doing it all in one go.

Any hints would be great, thanks!

Answer 1

Just adding to the wonderful answers already presented, but I wanted to clarify some things no one explicitly stated, just to be safe.

For starters, your values are being saved (though not the correct amount, not in the right locations, and not the right values, I'll explain that later). Your printf() statement simply isn't recognizing the values stored in each index of the array you have made. In your format string, you are using the %s format specifier. This would be okay if you were actually printing a human-readable ASCII code. The problem is you are printing the literal values 0 and 1, which map to "NULL" and "Start of heading", respectively.

The other answers have very good suggestions for how to map the numeric values to the ASCII values, but in theory, if you wanted to print the the indexes in your array as-is, you would need to print in the following way:

printf("The binary version of %d is ", input); for(int i = numDigits - 1; i >= 0; --i) printf("%d", result[i]);

In this case, you are printing each item in the array as numbers as opposed to characters.

The next thing I want to point out is your logic in the while loop. Your assignment to result[counter] is always baseTenNum % 2, so it would either store all 0s or all 1s depending on whether the number is always even or odd. Then on top of that, your realloc() is always resizing to a fixed size, as opposed to growing with each new character (as pointed out in another answer :-)).

Hope these help clarify some of your troubles!

Answer 2

Schwerm's answer is the best.
If you want to allocate memory on each iteration, realloc(result, counter + 2) will allocate one more byte as counter increases. The +2 provides a byte for the current character and one for the terminator.
To get the 1 and 0 from the modulus operation into characters that can be printed, add '0'.

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

char * convertToBinary(unsigned int baseTenNumber)
{
    int counter = 0;
    char *result = NULL;
    char *temp = NULL;
    int remainder = baseTenNumber;
    while(remainder)
    {
        if ( NULL == ( temp = realloc(result, counter + 2))) {
            fprintf ( stderr, "problem realloc\n");
            exit ( 0);
        }
        result = temp;
        printf("%d\n", remainder);
        result[counter] = remainder%2 + '0';
        remainder/=2;
        counter++;
    }
    result[counter] = 0;//terminate
    return result;
}

int main (void)
{
    unsigned int input = 0;

    do
    {
        printf("Enter an integer: ");
        scanf("%u", &input);
        if ( input) {
            char * result = convertToBinary(input);
            printf("The binary version of %d is %s\n", input, result);

            free(result);
        }
    } while(input);

    return 0;
}

Answer 3

There's a few ways to get an exact memory fit in C.

One is to precalculate exactly how much memory you need, which you could probably do here. But that's not always possible, and it's more code to write, and more bugs, and more runtime.

Another is to use realloc. Reallocating can get messy and expensive and it's a great way to invite bugs. Sometimes its necessary if you're working with an existing hunk of memory, like growing an array.

The other is to allocate a stack buffer large enough to hold any possible value, then use strdup to copy it to exactly the right amount of heap memory. This is great when you need to process something all at once and you're not sure how big it might be. For example, reading from a file.

// Allocate a big, reusable buffer
char line[BUFSIZ];
while( fgets(line, sizeof(line), fp) ){
    ...now you can slice and dice line...
}

We can do the same thing for your problem.

#include <stdio.h>
#include <string.h>

// The largest possible string to hold a binary integer
#define BINARY_INT_SIZE ((sizeof(int)*8) + 1)

// A cheap way to convert from integer 0/1 to char 0/1
const char BINARY_TO_CHAR[] = {'0', '1'};

char * convertToBinary(int number) {
    // Initialize the stack variable to hold the conversion   
    char result[BINARY_INT_SIZE] = {0};

    for( int counter = 0; number; counter++ ) {
        printf("%d %d %d\n", counter, number, number % 2);
        result[counter] = BINARY_TO_CHAR[number % 2];
        number/=2;
    }

    // Copy from the stack into exactly enough heap memory
    return strdup(result);
}

Stack memory is cheap and fast to allocate and discard. Much faster than reallocating heap memory over and over again in a loop. Using strdup guarantees the persisting heap memory is compact.