patterncMinor
C getline() implementation
Viewed 0 times
getlineimplementationstackoverflow
Problem
I'm practicing my C coding and I wanted to implement my own version of the
getline function in C for learning purposes. I'd like a review on coding style, correctness, improvements I can make for performance, and overall quality of the code. My main concerns are correctness and performance (in that order).#include
#include
#include
#define MAX_LINE_LENGTH 255
/* Retrieves a line of text from the stream provided
* and places it into @buf until a new line character is
* reached or the number of characters read is > @size - 1.
* This function will null-terminate the provided buffer.
*
* @param[in] -- stream -- the stream
* @param[in] -- buf -- a buffer big enough for @size chars.
* @param[in] -- size -- the maximum number of chars to read (must
* include room for a null terminator
* @return -- the number of characters read from the stream.
*/
size_t getline(FILE *stream, char *buf, size_t size)
{
size_t count = 0;
char c;
while ((c = (char)getc(stream)) != '\n' && count < size - 1) {
buf[count++] = c;
}
buf[count] = '\0';
return count;
}
int main()
{
char line[MAX_LINE_LENGTH];
while (true) {
size_t count = getline(stdin, line, MAX_LINE_LENGTH);
printf("The line gotten was \"%s\" and was %zu chars long.\n", line, count);
}
}Solution
-
Your
-
If
-
Your
-
Your
-
If
You can try this by pointing
-
Use a
This is how I would rewrite your function considering the above points:
Your
getline function looks more like a variant of fgets than getline.-
If
size == 0, size - 1 == SIZE_MAX, a very large number.-
Your
getline reads up to size bytes from the stream even though it only places only up to size - 1 into the buffer. It simply drops the last byte silently. You should switch the order of the loop condition:while (count < size && (c = (char)getc(stream)) != '\n') { ... }-
Your
getline also writes to memory outside the range defined by its parameters if size is 0, when it writes the terminating null byte.-
If
getc(stream) == EOF, which is an error condition, your getline function will keep trying to read from the stream, always "reading" EOF, placing (char) EOF (usually '\xff') into the buffer until the end of the buffer is reached.You can try this by pointing
stdin at an empty file.-
Use a
for loop instead of a while loop, if you're using an iteration counter. It'll be much clearer, what is incremented where and how.This is how I would rewrite your function considering the above points:
ssize_t fgets(FILE *stream, char *buf, size_t size)
{
if (size == 0)
return 0;
size_t count;
int c = 0;
for (count = 0; c != '\n' && count < size - 1; count++)
{
c = getc(stream);
if (c == EOF) {
if (count == 0)
return -1;
break;
}
buf[count] = (char) c;
}
buf[count] = '\0';
return (ssize_t) count;
}Code Snippets
while (count < size && (c = (char)getc(stream)) != '\n') { ... }ssize_t fgets(FILE *stream, char *buf, size_t size)
{
if (size == 0)
return 0;
size_t count;
int c = 0;
for (count = 0; c != '\n' && count < size - 1; count++)
{
c = getc(stream);
if (c == EOF) {
if (count == 0)
return -1;
break;
}
buf[count] = (char) c;
}
buf[count] = '\0';
return (ssize_t) count;
}Context
StackExchange Code Review Q#119219, answer score: 6
Revisions (0)
No revisions yet.