Functionality
Error: input(stdin, 0)
could quickly return NULL
as code incorrectly assumes a NULL
return only means out-of-memory.
// if (!input_str) return NULL;
if (!input_str && size > 0) return NULL;
Error: Memory leak @Daniel Jour@Daniel Jour. Should realloc()
fail, code should free input_str
.
size = size ? size*2 : 1;
void *new_ptr = realloc(input_str, size);
if (new_ptr == NULL) {
free(input_str);
return NULL;
}
Weakness: Code lacks EOF detection. If only '\n'
or EOF
is read, both cases return a string of ""
. Code needs to differentiate. Various approaches exists. With this code, recommend returning NULL
when only EOF
was read. Calling code could distinguish between OOM and EOF
occurrence, by calling feof()/ferror()
.
Memory leak with return realloc(input_str, len);
on rare failure.
// return realloc(input_str, len);
void *new_ptr = realloc(input_str, len);
return new_ptr ? new_ptr : input_ptr;
Soap box
Disagree with unlimited input strategy as it gives external input the ability for code to consume insane amount of resources without control - ripe for hackers. Better to pass in an upper bound.
Design
size
parameter seems of little use. It only provides an initial size and is ignored for subsequent re-allocations. Recommend removing it. Just start with some small size.
A better use would be to use size_t *size_read
to report that number of characters read. This has an advantage over using strlen()
on the return value as 1) the length does not need O(n) computation and 2) handles the corner case should code read null characters which strlen()
will not detect.
IMO, code should not attempt to read unless the needed memory has first been successfully allocated. Also on allocation failure, perhaps it would be better to return the partial read line. Of course, conveying that information is another issue. IAC, this allows the calling routine more control on error handling.
Performance
Use exponential growth in re-allocation rather than linear. This is well answered by @mleyfman@mleyfman
With exponentiation strategy, it makes even more sense then to do a final "right-size " re-allocation, which OP did. With linear +16
re-allocation, it has reduced value to the point that it is unnecessary.
Minor: Check against the more common '\n'
first.
// while ((c = fgetc(in)) != EOF && c != '\n')
while ((c = fgetc(in)) != '\n' && c != EOF)