Thread: Simulating the string class - your comments on this, please

Code:

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

int main(void)
{
    int i, c;
    char *input;

    i = 0;

    printf("Type something: ");
    c = getchar();

    input = malloc(1 * sizeof(c));
    if (input == NULL)
        return 1;

    input[i++] = c;

    while ((c = getchar()) != EOF && c != '\n') {
        if ((realloc(input, 1 * sizeof(c))) == NULL)
            return 1;
        input[i++] = c;
    }

    input[i] = '\0';

    printf("%s\n", input);

    free(input);

    return 0;
}

Thank you.

Can you elaborate, please?

You never want to use realloc without assigning the return value to something. Suppose you want to realloc the array pointed to by x, to the size y. The expression realloc(x, y) will return a pointer to an array of size y whose first n elements (where n was the size of the array pointed to by x) are equal to those of x. If y > x, then this might be a new location! Which means that the pointer x is no longer pointing to a valid memory address.

As a result, you generally want to use x = realloc(x, y);

What bothers me is that in your case, you are not actually changing the size of the array. You malloc'd your array to size 1 * sizeof(c) and you're realloc'ing your array to the size 1 * sizeof(c). Since c is an int, and since ints are usually four bytes wide nowadays, your array is going to be four characters wide. Since you're not changing the size of the array, input will still be pointing to a valid memory address (since a new one won't be used). This might or might not be guaranteed by C standards.

What then happens is that you might increment i four times. And if that happens, then you've incremented i into uncharted territory, and you're going to be writing over uncharted memory.

I recommend re-reading up on how realloc works.

Is this correct now?

Code:

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

int main(void)
{
    int i, c;
    char *input;

    i = 0;

    printf("Type something: ");
    c = getchar();

    input = malloc(1 * sizeof(c));
    if (input == NULL)
        return 1;

    input[i++] = c;

    while ((c = getchar()) != EOF && c != '\n') {
        input = realloc(input, 1 * sizeof(*input) + 1 * sizeof(c));
        if (input == NULL)
            return 1;
        input[i++] = c;
    }

    input[i] = '\0';

    printf("%s\n", input);

    free(input);

    return 0;
}

No. sizeof(*input) will always be 1, even if you're previously allocated more memory for the input pointer. You need to keep track of the size of the buffer in a separate variable. input is just a pointer to a single byte (char), it has no concept of size beyond that.

Also, although getchar returns an int, when you assign it to a deference of input, it becomes a char, since input is a pointer to char.

Hum, so the variable i would be the perfect argument for the realloc function, right? Something like this:

Code:

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

int main(void)
{
    int i, c;
    char *input;

    i = 0;

    printf("Type something: ");
    c = getchar();

    input = malloc(1 * sizeof(c));
    if (input == NULL)
        return 1;

    input[i++] = c;

    while ((c = getchar()) != EOF && c != '\n') {
        input = realloc(input, i + 1);
        if (input == NULL)
            return 1;
        input[i++] = c;
    }

    input[i] = '\0';

    printf("%s\n", input);

    free(input);

    return 0;
}

Probably. A few more points:

You are still originally malloc'ing sizeof(int), but you only need 1 byte for the first char.

malloc/realloc are relatively expensive operations, so in the real world, you would likely want to malloc/realloc larger chunks at a time as you need them, instead of each time a new character needs appending.

Follow quzah's advice to use a different variable for the return value of realloc to the thing you are trying to realloc. Although you are ending the program as soon as realloc fails, it is good practice to call free on the original untouched pointer before failing. This would be especially appropriate if you had this in a function, rather than a whole program that terminates on realloc failure.

(edit: last paragraph becomes semi-redundant, I didn't see quzah's last post before submitting)

Originally Posted by Dave_Sinkula

[edit]And here generally I draw a distinction -- sizeof(char) will always be 1, sizeof *input may not.

I probably wasn't explaining very clearly, but I meant that sizeof(*input) will always be the size of what input is a pointer to, in this case, a char.

The original poster's code suggested he was under the impression that as his "array" grew, sizeof(*input) would evaluate to a larger and larger value.

Obviously, if the code is changed and input is changed to point to something other than a char, then sizeof(*input) will be different.

Thanks Dave. So, does it look good now?

Code:

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

int main(void)
{
    int i, c;
    char *input;
    char *temp;

    i = 0;

    printf("Type something: ");
    c = getchar();

    input = malloc(1 * sizeof(c));
    if (input == NULL)
        return 1;

    input[i++] = c;

    while ((c = getchar()) != EOF && c != '\n') {
        temp = realloc(input, i + 1);
        if (temp == NULL) {
            free(input);
            return 1;
        }

        input = temp;
        input[i++] = c;
    }

    input[i] = '\0';

    printf("%s\n", input);

    free(input);
    free(temp);

    return 0;
}