patterncMinor
Reading unlimited input strings in C
Viewed 0 times
inputreadingstringsunlimited
Problem
The idea is to write a function that reads string of unknown size from
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?
stdin, 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
#include
#include
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;
}Solution
Here are a few comments on your function:
-
Firstly,
-
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 by doubling its size (instead of adding 11).
-
As you have it, your
-
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.
-
Your reallocation call leaks memory if it fails to allocate - you need a temporary variable to hold the return value from realloc:
-
Your loop exits when
You must call
-
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 a very big test file as input).
-
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 by doubling its size (instead of adding 11).
-
As you have it, your
fgets call overwrites the char after the end of the buffer with \0. You should pass the actual size of the buffer to fgets. -
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 when
fgets returns NULL, but you do not check for an error.You must call
ferror 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 a very big test file as input).
Code Snippets
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;
}
...
}size *= 2;
if ((p = realloc(buf, size)) == NULL) {
break; // and then return an error if `p == NULL` or `ferror`
}
buf = p;
p += len;Context
StackExchange Code Review Q#31095, answer score: 3
Revisions (0)
No revisions yet.