C getline() implementation

Question

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 <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#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);
 }
}

• Jasen: calling this getline is probably a bad idea, it's nothing like getline().

Answer 1

• 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;
}

Answer 2

There should be some error-checking done here. For instance, it should return an ssize_t, allowing it to return a negative value to indicate an error (anything at or above 0 can be seen as a valid count). It's also the return type of the actual function.