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.

• chux: David542, how do you want to distinguish between end-of-file and reading only "\n"?
• chux: What is the goal with code like if (!started && isspace(c));?
• David542: @chux-ReinstateMonica to skip leading whitespace (\s being used to mean space): \s\s\s Hello, chux --> Hello, chux. But it looks like that's bad code and there's an error within it.
• chux: David542 Why skip all leading white-space and not symmetrically all trailing white-space (aside from 1 '\n') as with " ABC \n"?
• David542: @chux-ReinstateMonica no reason, I suppose doing both would be better as you suggest.

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

• David542: thanks for this. Could you describe how it might be split up into skip_initial_spaces and actually_read_data in a couple lines of code or so?
• vnp: @David542 see edit.
• chux: OP's code only reads up to 1 line. This code may read multiple lines with skip_leading_whitespaces().
• chux: actually_read_data() does not use limit. As you say " something along these lines:".

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.

• David542: thanks. Would you want to post an updated version of the code with how it can be improved?
• chux: @David542 Still waiting on question.