Limit fgets input to up to a certain number of characters

Question

This question is an extension of this other question from Stackoverflow.

The objective is to limit the input of characters read by fgets to up to a certain number of characters. And in the following code, that I've come up with as a solution, we are working with a limit of 10 characters as an example:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main()
{
    char v[12];
    
    printf("Type a word with up to 10 chracters:\n");
    //Loop checks if fgets succeeded and if there is '\n' in the string:
    while (!fgets(v, 12, stdin) || strcspn(v, "\n") == 11){
        if(feof(stdin)) {
             printf("Error: End of file reached.\n");
             exit(EXIT_FAILURE);
        }
        printf("Error: The word is longer than 10 characters, try again.\n");
        for(int ch=getchar(); ch != '\n' && ch != EOF; ch=getchar());
        printf("Type a word with up to 10 chracters:\n");
    }
    v[strcspn(v, "\n")] = '\0';
    
    printf("%s\n", v);

    return 0;
}

It works as intended. However @chux-ReinstateMonica has pointed out that there may still be corner weaknesses and possible improvements to be made to this solution.

So, how can this code/solution be improved?

Answer 1

This would obviously be better as a function, so we can call with different parameters.

Instead of repeating the constant 12 in the fgets() call, we could simply use sizeof v. Then it remains consistent if we change v.

Instead of strcspn() with only one character, I'd prefer strchr(). Conveniently, that returns a null pointer if not found, so we don't need to calculate the length to search. And we shouldn't repeat the search to replace the newline with null character.

Error messages should go to stderr, not stdout.

We can skip the rest of line with a simple scanf("%*[^\n]") (note the * to suppress assignment). That would replace the ch loop.

Answer 2

Does not detect input errors on stdin

When fgets() returns NULL due to an input error, code simply loops when a loop exit is more common. If the input error is permanent, code is stuck in a infinite loop.

Avoid naked magic numbers

Rather than 12, 11, etc, use a #define BUF_N 12 and code accordingly.

// while (!fgets(v, 12, stdin) || strcspn(v, "\n") == 11){
while (!fgets(v, BUF_N, stdin) || strcspn(v, "\n") == BUF_N - 1){

Good use of int

//  vvv --- proper type to save the typical 257 different responses from `fgetc()`
for(int ch=getchar(); ch != '\n' && ch != EOF; ch=getchar());

Spell check

chracters --> characters

---

Sample code

Following OP's style, as a function returning a flag rather than printing errors:

// Look for an input <= N-2 non-\n characters.
// Return error/EOF flag
bool OP_readline(char *v, size_t n, const char *re_prompt) {
  char *fgets_retval;
  size_t len = 0;
  while ((fgets_retval = fgets(v, n, stdin)) != NULL && (len = strcspn(v, "\n")) + 1 == n) {
    int ch;
    while ((ch = getchar()) != '\n' && ch != EOF) {
      ;
    }
    if (ch == EOF) {
      return true; // Error or EOF;
    }
    if (re_prompt) {
      fputs(re_prompt, stdout);
      fflush(stdout):
    }
    len = 0;
  }
  v[len] = 0;
  return fgets_retval == NULL;
}

Other improvements

1. Better code would not need a buffer of +1 size. This allows one to read directly into an array of size N without providing a N+1 size array just for the benefit the read function. Detecting long lines is the read function's problem and should not oblige the caller to provide a +1 buffer to solve it.

2. Null character input can fool above code a bit, but leave that for another day.

3. Detection/Handling of extreme n. Should be in the range 2 <= n <= INT_MAX, or some other well thought out limits.

4. Consider a prompt parameter too.

Answer 3

Really nice style; super clear. I'll try not to duplicate existing and really good answers. What I think you intend to do is a kind of state machine looking something like this:

The error, in this case, includes stdin EOF before return was pressed. However, you are only checking for EOF and ignoring other errors, as pointed out. I believe that the fgets, getchar, and error states will probably make your code clearer.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h>
#include <errno.h>
#include <limits.h>

// user-defined errno values; should all be negative to avoid conflicts
enum { NOT_POSIX_COMPLIANT = -2, EOF_ERROR };

/** Blocks, prompting the user again and again. If true, `v` is full up to
 `v_size`, including "". If false, `errno` is set to and expanded range. `v`
 must be capable of holding `v_size` bytes. */
static int prompt(char *const v, const size_t v_size) {
    size_t len;
    int ch;
    assert(v && v_size > 2 && v_size <= INT_MAX);
    //Loop checks if `fgets` succeeded and if there is '\n' in the string:
    for( ; ; ) {
        printf("Type a word with up to %zu chracters:\n", v_size - 2);
        if(!fgets(v, (int)v_size, stdin)) goto eof;
        if(v[len = strcspn(v, "\n")] == '\n') return v[len] = '\0', 1;
        //fprintf(stderr, "Error: input line too long.\n");
        fprintf(stderr, "Discarding malformed line.\n");
        while(ch = getchar(), ch != '\n') if(ch == EOF) goto eof;
    }
eof:
    if(feof(stdin)) errno = EOF_ERROR; // in this use-case, it is an error
    else if(!errno) errno = NOT_POSIX_COMPLIANT; // "Extension to the ISO"
    return 0;
}

int main(void) {
    char v[12];

    if(!prompt(v, sizeof v)) goto error;
    printf("%s\n", v);

    return 0;

error:
    switch(errno) {
    case NOT_POSIX_COMPLIANT:
        fprintf(stderr, "An unknown read error occurred.\n"); break;
    case EOF_ERROR:
        fprintf(stderr, "Premature EOF.\n"); break;
    default:
        perror("input");
    }
    return EXIT_FAILURE;
}

I've re-arraged some stuff using what it says in the man pages about errors to match the diagram. It's hard to do interactive programmes in purely ISO C. I would consider, depending on your application, returning failure immediately upon mis-formed input. Especially if you think there is a good reason to pipe data to stdin.

Answer 4

IMO a much simpler solution is to use POSIX getline(). And if your platform doesn't provide getline(), there are more than a few open-source implementations available.

Bare code to illustrate the processing in a smaller number of lines (as in without scroll bars...):

int main()
{
    char *line = NULL;
    size_t len = 0;
    for ( ;; )
    {
        printf( "Type a word with up to 10 characters:\n" );
        ssize_t result = getline( &line, &len, stdin );
        if ( result < 0 )
        {
            exit( EXIT_FAILURE );
        }

        line[ strcspn( line, "\n" ) ] = '\0';
        if ( strlen( line ) <= 10 )
        {
            break;
        }

        printf( "Error: The word is longer than 10 characters, try again.\n" );
    }

    printf( "Your input: %s\n",  line );    
    return 0;
}

The use of strcspn() and strlen() could be made more efficient so the line only gets traversed once, but we're dealing with user input here so code clarity overrides almost every performance consideration.

With full error checking:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main()
{
    char *line = NULL;
    size_t len = 0;

    for ( ;; )
    {
        printf( "Type a word with up to 10 characters:\n" );

        ssize_t result = getline( &line, &len, stdin );
        if ( result < 0 )
        {
            if ( feof( stdin ) )
            {
                fprintf( stderr, "EOF reached\n" );
            }
            else
            {
                perror( "getline(..., stdin)" );
            }

            free( line );
            exit( EXIT_FAILURE );
        }

        line[ strcspn( line, "\n" ) ] = '\0';

        if ( strlen( line ) <= 10 )
        {
            break;
        }

        printf("Error: The word is longer than 10 characters, try again.\n");
    }

    printf( "Your input: %s\n",  line );

    free( line );

    return 0;
}

As a function:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char *getLine( unsigned int maxLen )
{
    char *line = NULL;
    size_t len = 0;

    // sanity check passed variable
    if ( 0 == maxLen )
    {
        return( NULL );
    }

    for ( ;; )
    {
        printf( "Type a word with up to %u characters:\n", maxLen );

        ssize_t result = getline( &line, &len, stdin );
        if ( result < 0 )
        {
            if ( feof( stdin ) )
            {
                fprintf( stderr, "EOF reached\n" );
            }
            else
            {
                perror( "getline(..., stdin)" );
            }

            free( line );
            return( NULL );
        }

        line[ strcspn( line, "\n" ) ] = '\0';

        if ( strlen( line ) <= maxLen )
        {
            break;
        }

        printf( "Error: The word is longer than %u characters, try again.\n",
            maxLen );
    }

    printf( "Your input: %s\n",  line );

    return( line );
}