2
\$\begingroup\$

This is meant to be a drop in replacement for getline as it's not crossplatform. I am trying to do as minimal work as possible in here including only calling strlen once at the very end to calculate the size of the final block.

returns -1 for error, 0 for EOF, 1 for success

The way it works it by setting the last two bytes in the array as [0, 1] and if the buffer is filled it will look like [X,0], X being either LF where we are done, or something else meaning we need to realloc and loop again unless we are at eof.

int
fgets_line(char **line, size_t *capacity, size_t *length, FILE *stream) {
  if (!*line && !(*line = malloc(*capacity = 64)))
    return *capacity = 0, -1;
  (*line)[0] = 0;
  if (length)
    *length = 0;
  char *position = *line;
  while (1) {
    (*line)[*capacity - 1] = 1;
    (*line)[*capacity - 2] = 0;
    if (fgets(position, *line + *capacity - position, stream)) {
      if ((*line)[*capacity - 1] == 0
       && (*line)[*capacity - 2] != '\n') {
        size_t new_capacity = *capacity * 2;
        char *tmp = realloc(*line, new_capacity);
        if (tmp) {
          position = tmp + *capacity - 1;
          *line = tmp;
          *capacity = new_capacity;
        } else return -1;
      } else goto success;
    } else if (position != *line) {
      if (feof(stream))
        goto success;
      return -1;
    } else return 0;
  }
  success:
  if (length)
    *length = position - *line + strlen(position);
  return 1;
}

usage is similar to getline

int main() {
  char *line = NULL;
  size_t capacity = 0, length = 0;
  int result = 0;
  while ((result = fgets_line(&line, &capacity, &length, stdin)))
    printf("---Capacity: %-8zuLength: %-8zu---\n%s------\n", capacity, length, line);
}
\$\endgroup\$
3
\$\begingroup\$
  • Do not put the sentinel characters in every iteration. Set them before the loop, and after a successful reallocation.

    Also, (*line)[*capacity - 2] = 0; is redundant. Indeed, if (*line)[*capacity - 1] remains 1 we don't care, and if it turns to 0 then (*line)[*capacity - 2] has been overwritten anyway.

  • goto is totally unwarranted. break works very well.

  • Do not handle fgets failures inside the loop. Break immediately.

  • Using position as an index, rather than the pointer, seems cleaner. Consider

      fgets(*line + position, *capacity - position, stream)
    

    etc.

\$\endgroup\$
2
\$\begingroup\$

Error: Mishandles input errors

When fgets() returns NULL due to an input error, this function might return 0 (for end-of-file).

Lack of in code documentation

A declaration like int fgets_line(char **line, size_t *capacity, size_t *length, FILE *stream); deserves documentation describing the goals and function limitations. (e.g. a .h file.) Do not assume users want to dissect the function source code to learn basic functionality.

Minimum work

"minimal work as possible" --> User I/O is a sink-hole of time. A linear extra call to strlen() will not be noticed.

Pedantic: Reliance on not reading a null character

Code performs incorrectly with *length = position - *line + strlen(position); should fgets() read a null character.

Either adjust code to detect reading a null character (not easy) or consider dropping the length parameter. Calling code can use strlen() if desired.

Pedantic: *capacity not validated

If fgets_line() is called with a wee *capacity (like 0 or 1), (*line)[various] risks accessing the array out of bounds. Take care in assuming how the caller set up the buffer pointer and size. It may have been right-sized after a prior read.

Pedantic: Out of int range

fgets() uses an int for the size, so *line + *capacity - position risks out-of-range conversion.


Design

I am not a fan of giving the user the ability to overwhelm memory resources and prefer a sane upper bound to allocation size.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks for pointing out that fgets skips over NULL bytes, that hadn't even occurred to me. This function is supposed to return 0 for EOF, but I guess a valid point could be made that 0 should be success, -1 for EOF, and 1 for error. I did notice the out of int range problem when compiling with MSVC, and planned to solve it. \$\endgroup\$
    – Anotra
    Mar 30 at 19:00
  • \$\begingroup\$ @Anotra fgets() does not skip over null characters. It reads and saves them like any other non-'\n' character. The trouble is that it is hard to detect them for the caller. \$\endgroup\$ Mar 30 at 19:04
  • \$\begingroup\$ right, my wording was bad. That's what I mean \$\endgroup\$
    – Anotra
    Mar 30 at 19:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.