Reading unlimited input strings in C

Question 1

The idea is to write a function that reads string of unknown size fromstdin, keep it in dynamically allocated array of unknown size and return a copy of that array.

Is this the right approach? Could I be more clear? Would reading the input char-by-char be better than getting bigger chunks at once?

#include <stdlib.h>#include <string.h>#include <stdio.h>char * read_string(void){    int buff_size = 11; /* size of temporary buffer */    char temp[11]; /* temporary input buffer */    char *output; /* pointer to allocated memory */    char *copy; /* copy of input string to be returned */    char *iterator; /* pointer to beginning of an array, used when searching for \n in input string */    output = malloc(10*sizeof(char)); /* allocates 10 bytes for first part of input */    if(output == NULL)        return NULL;    size_t size = 10*sizeof(char); /* keeps track of actual size of output */    //*output = '\0'; /* places NUL at the beginning of output, to make it empty string */    while(fgets(temp, buff_size,stdin) != NULL) /* read  max 10 chars from input to temp */    {        strcat(output,temp); /* append read input to output, without \0 char */        if(strlen(temp) != 10 || temp[9] == '\n') /* if buffer wasn't full or last char was \n-stop because it reached the end of input string */            break;        size += buff_size - 1; /* if didn' reach end of input increase size of output*/        output = realloc(output,size);        if(output == NULL)        {            return NULL;        }    }    iterator = output; /* search for \n in output and replace it with \0 */    while(*iterator != '\n')        iterator++;    *iterator='\0';    copy=malloc(strlen(output) + 1); /* allocate memory for copy of output +1 for \0 char */    if(copy == NULL)        return NULL;    strcpy(copy, output); /* strcpy output to copy */    free(output);  /* free memory taken by output */    return copy;}

Question 2

I edited your code to make it more readable. Also, note thatsizeof(char) is 1 by definition, so multiplying by it is redundant.

Question 3

It would be wise to check if the reallocation of the memory worked.

Also, this:

copy=malloc(strlen(output)+1); /* allocate memory for copy of output +1 for \0 char */if(copy==NULL)  return NULL;strcpy(copy, output); /* strcpy output to copy */free(output);  /* free memory taken by output */return copy;

Why making a copy ofoutput, freeingoutput, and then returningcopy? Why not just returningoutput?

Here is a chunk of code I wrote a few years ago, which I find a bit simpler than your code:

char * read_string(void) {  char *big = NULL, *old_big;  char s[11] = {0};  int len = 0, old_len;  do {    old_len = len;    old_big = big;    scanf("%10[^\n]", s);    if (!(big = realloc(big, (len += strlen(s)) + 1))) {      free(old_big);      fprintf(stderr, "Out of memory!\n");      return NULL;    }    strcpy(big + old_len, s);  } while (len - old_len == 10);  return big;}

This was just a learning example, which I have never really used. In practice, I'd suggest working with a largers (char s[1025],%1024[^\n] inscanf, andlen - old_len == 1024 in the condition of thedo...while loop) to reduce the number ofrealloc calls which can be very slow if the user gets unlucky. This also answers your last question: char-by-char might get very slow and is in no way better than chunks.

Question 4

This is also a learning example, copy is part of assignment.

Question 5

@zubergu Are you sure it was meant to be inside the function? Maybe it was meant to be called like this:output = read_string(); copy = malloc(strlen(output)+1); strcpy(copy, output); free(output);.

Question 6

Btw, I've edited reporting of an error in my code.

Question 7

task: "Write a function that reads a string from the standard input and returns a copy of the string in dynamically allocated memory. The funcion may not impose any limit on the size of the string being read. " English isn't my native l. but think i got it correct.

Question 8

ATM I'm really proud that you didn't find any serious flaws, that would prove me entirely wrong and my code to be gibberish.

Question 9

Here are a few comments on your function:

Firstly,read_line seems a more accurate name, as you read up until the next \n.
Reading 10 chars at a time is an odd. Unless that is part of the requirement I think you should allocate a bigger buffer and read as many chars as fit, extending the buffer each time round the loop bydoubling its size (instead of adding 11).
As you have it, yourfgets call overwrites the charafter the end of the buffer with \0. You should pass the actual size of the buffer tofgets.
Your fgets/strcat/strlen sequence is inefficient. You should be reading directly into the target buffer rather than reading and then copying. And the strcat call has to traverse the whole length of the accumulated string to concatenate the new part.
```
char *p = buf;size_t len = 0;while (fgets(p, size - len, stdin) != NULL) {    len += strlen(p);    if (buf[len-1] == '\n') {        buf[len-1] = '\0';        break;    }    ...}
```

Your reallocation call leaks memory if it fails to allocate - you need a temporary variable to hold the return value from realloc:

size *= 2;if ((p = realloc(buf, size)) == NULL) {    break; // and then return an error if `p == NULL` or `ferror`}buf = p;p += len;

Your loop exits whenfgets returns NULL, but you do not check for an error.
You must callferror to be sure that an error did not occur.
And your looping at the end to find the \n seems wasteful when you have already located the end of line/string in the main loop.
From the task statement you posted, copying the dynamic string is not required.

Note that reading character at a time is not necessarily a slower solution. As you are doing it (with all that the copying and string traversal), I would expect a well written character-at-a-time function to be faster. I encourage you to try both and compare the execution speed (using avery big test file as input).

Vedran Šego 2,1311 gold badge17 silver badges23 bronze badges · Accepted Answer · 2013-09-11 14:36:04Z

It would be wise to check if the reallocation of the memory worked.

Also, this:

copy=malloc(strlen(output)+1); /* allocate memory for copy of output +1 for \0 char */if(copy==NULL)  return NULL;strcpy(copy, output); /* strcpy output to copy */free(output);  /* free memory taken by output */return copy;

Why making a copy ofoutput, freeingoutput, and then returningcopy? Why not just returningoutput?

Here is a chunk of code I wrote a few years ago, which I find a bit simpler than your code:

char * read_string(void) {  char *big = NULL, *old_big;  char s[11] = {0};  int len = 0, old_len;  do {    old_len = len;    old_big = big;    scanf("%10[^\n]", s);    if (!(big = realloc(big, (len += strlen(s)) + 1))) {      free(old_big);      fprintf(stderr, "Out of memory!\n");      return NULL;    }    strcpy(big + old_len, s);  } while (len - old_len == 10);  return big;}

This was just a learning example, which I have never really used. In practice, I'd suggest working with a largers (char s[1025],%1024[^\n] inscanf, andlen - old_len == 1024 in the condition of thedo...while loop) to reduce the number ofrealloc calls which can be very slow if the user gets unlucky. This also answers your last question: char-by-char might get very slow and is in no way better than chunks.

This is also a learning example, copy is part of assignment.
@zubergu Are you sure it was meant to be inside the function? Maybe it was meant to be called like this:output = read_string(); copy = malloc(strlen(output)+1); strcpy(copy, output); free(output);.
task: "Write a function that reads a string from the standard input and returns a copy of the string in dynamically allocated memory. The funcion may not impose any limit on the size of the string being read. " English isn't my native l. but think i got it correct.
ATM I'm really proud that you didn't find any serious flaws, that would prove me entirely wrong and my code to be gibberish.

Movatterモバイル変換

Stack Exchange Network

Reading unlimited input strings in C

2 Answers2

You mustlog in to answer this question.

Related

Hot Network Questions

Subscribe to RSS