HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

C getline() implementation

Submitted by: @import:stackexchange-codereview··
0
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 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.