Getline function

Question

The following is my attempt at writing a getline() function that makes it a bit easier to work with. I suppose an easier way would be to use fgets, but hopefully the following is good enough for some feedback:

#include <stdio.h>
#include <stdbool.h>
#include <ctype.h>

#define TITLE_MAX 50

void mygets(char buffer[], size_t limit)
{
    // first non-space char until newline or EOF, 
    // eating any extra chars in buffer
    // will add \0 at the end, so will get up to N-1 chars

    int c, idx=0;
    bool started=false;
    while ((c=getchar()) != EOF && c != '\n')
    {
        if (!started && isspace(c));
        if (idx < limit-1) {
            started = true;
            buffer[idx++] = c;
        }
    }
    buffer[idx] = '\0';
}

int main(void)
{
    char tbuffer[TITLE_MAX];
    while (true) 
    {
        printf("Enter the name of the film (empty line to stop)\n");
        mygets(tbuffer, TITLE_MAX);
        if (*tbuffer=='\0') break;
        printf("The title is: %s\n", tbuffer);
    }
}

Working example on onlinegdb

How does it look? How could it be improved? Additionally, how are comments usually done in C code? It is usually within the function braces? Before the function? etc.

Answer 1

• You should be getting a warning (at least I do):

  warning: if statement has empty body [-Wempty-body]
    if (!started && isspace(c));
                               ^
  

  This line does absolutely nothing. Judging from the comment, you probably meant

    if (!started && isspace(c))
        continue;
  

• The loop looks overcomplicated. Its logic cries to be split into two independent actions: skip_initial_spaces and actually_read_data, performed sequentially (again, assuming that I read the comment correctly).

  Factoring them out into the functions of their own will also make your code comply with a single responsibility principle.

• I strongly advise against void functions. Do not discard the information you have already computed; it is very likely the caller will need it. In this case, returning idx will spare the caller an additional call to strlen.

EDIT. On splitting up the functionality, something along these lines:

int skip_leading_whitespaces()
{
    int c;
    while (((c = getchar()) != EOF) && isspace(c)) {
    }
    return c;
}

int actually_read_data(int c, char buffer[], size_t limit)
{
    int idx = 0;
    while ((c != EOF) && (c != '\n')) {
        buffer[idx++] = c;
        c = getchar();
    }
    buffer[idx] = 0;
    return idx;
}

int mygets(char buffer[], size_t limit)
{   
    int c = skip_leading_whitespaces();
    return actually_read_data(c, buffer[], limit);
}

Answer 2

Incorrect code to skip leading white space

Asymmetrically attempts to skip leading white-spaces but not all trailing ones

Mixed types

Rather than size_t limit, int idx. Use the same type. Recommend size_t

Edge cases behavior woe

idx < limit-1 is like idx < SIZE_MAX when limit == 0. Better as idx + 1 < limit.

No return

No clear what to distinguish end-of-file from reading "\n".

---

I'd set aside the skip leading white-space goal for now.