3

I'm learning C programming and I have to implement a program that read an input string of of unknown size. I wrote this code:

int main() {
    char *string;
    char c;
    int size = 1;

    string = (char*)malloc(sizeof(char));

    if (string == NULL) {
        printf("Error.\n");
        return -1;
    }
    printf("Enter a string:");
    while ((c = getchar()) != '\n') {
        *string = c;
        string = (char*)realloc(string, sizeof(char) * (size + 1));
        size++;
    }
    string[size - 1] = '\0';

    printf("Input string: %s\n", string);

    free(string);
    return 0;
}

But the last printf doesn't show the whole string but only the last char. So if I enter hello, world the last printf prints d.

After a little research I tried this code and it works! But I don't get the difference with mine.

I hope I made myself clear, thank you for your attention.

Community
  • 1
  • 1
Ker
  • 33
  • 2
  • Note that incrementing the size of the string by one byte at a time can lead to poor performance. Generally, you're best off doubling the size of the string, or thereabouts. If it is dramatically too large at the end (say you got to 2048 bytes of string, but only use 1080 bytes), you could use another `realloc()` to shrink it back down to size. But that's relatively unlikely to be a major problem. – Jonathan Leffler Jan 04 '16 at 03:02
  • @JonathanLeffler: while I agree with your remark, the inefficiency of the `realloc` scheme is the least of the OPs problem as it stands. – chqrlie Jan 04 '16 at 03:17
  • 1
    @chqrlie: that's why it is a comment, not an answer. – Jonathan Leffler Jan 04 '16 at 04:53
  • 1
    Do not cast the result of `malloc` & friends in C. – too honest for this site Jan 04 '16 at 07:07

4 Answers4

5

In your version of the code you assign the newly read character, c, to string, using:

*string = c;

*string points at the beginning of the string so you keep replacing the first character of string with the newly read character.

The code you linked to does the following:

str[i] = c

Basically, it is assigning the character to the end of the string, using the index i.

In your version of the code you could use size - 1 instead of i.

mttrb
  • 8,297
  • 3
  • 35
  • 57
3

Try changing:

*string = c;

To:

string[size-1] = c;

That way you won't just overwrite the first character each time.

Tom Karzes
  • 22,815
  • 2
  • 22
  • 41
3

There are several problems with your code:

  • you store all characters into the first byte of allocated memory
  • you read characters into a char variable, you cannot correctly test for EOF.
  • you will run an infinite loop, allocating all available memory and finally crash if standard input does not contain a '\n', such as redirecting from an empty file.
  • less important, you reallocate the buffer for each byte read, inefficient but can be optimized later.

Here is a corrected version:

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

int main() {
    char *string;
    int c;
    int len = 0;

    string = malloc(1);
    if (string == NULL) {
         printf("Error.\n");
         return -1;
    }
    printf("Enter a string:");
    while ((c = getchar()) != EOF && c != '\n') {
        string[len++] = c;
        string = realloc(string, len + 1);
        if (string == NULL) {
            printf("cannot allocate %d bytes\n", len + 1);
            return -1;
        }
    }
    string[len] = '\0';

    printf("Input string: %s\n", string);
    free(string);
    return 0;
}

Regarding your question about the difference with the linked code, it uses the same method, has one less bug but also one more bug:

  • it stores the character in the appropriate offset in str.
  • it runs an infinite loop if the input file does not contain a '\n', same as yours.
  • it invokes undefined behavior because c is not initialized for first test.
chqrlie
  • 131,814
  • 10
  • 121
  • 189
1

try

*(string + size - 1) = c;

maybe this helps

kaitian521
  • 548
  • 2
  • 10
  • 25