3
\$\begingroup\$

What do you think of the following function to offer an alternative to fgets, looking for insights about portability, safety, readability and performance:

#include <stdio.h>
#include <errno.h>

int sc_gets(char *buf, int n)
{
    int count = 0;
    char c;

    if (__glibc_unlikely(n <= 0))
        return -1;

    while (--n && (c = fgetc(stdin)) != '\n')
        buf[count++] = c;
    buf[count] = '\0';

    return (count != 0 || errno != EAGAIN) ? count : -1;
}

#define BUFF_SIZE 10

int main (void) {
    char buff[BUFF_SIZE];

    sc_gets(buff, sizeof(buff)); // how to use
    printf ("%s\n", buff);

    return 0;
}

NB: I have already read fgets() alternative which was interesting but my code is different.


Second version thanks to the comments and vnp's insight:


#include <stdio.h>
#include <errno.h>

long sc_gets(char *buf, long n)
{
    long count = 0;
    char c;

    while (--n && (c = fgetc(stdin)) != '\n') {
        if (c == EOF)
            break;
        buf[count++] = c;
    }
    buf[count] = '\0';

    return (count != 0 || errno != EAGAIN) ? count : -1;
}

#define BUFF_SIZE 1024

int main (void) {
    char buff[BUFF_SIZE];
    sc_gets(buff, sizeof(buff));
    printf ("%s\n", buff);

    return 0;
}
\$\endgroup\$
9
  • \$\begingroup\$ If you are going to use implementation specific API __glibc_unlikely() it is probably best to wrap them in a macro to provide alternatives for implementations that don't have them. \$\endgroup\$ Mar 27, 2021 at 17:50
  • \$\begingroup\$ I already checked about them, there is no need to replace them as "the platforms that don't support them just define them to expand to empty strings. " stackoverflow.com/questions/109710/… \$\endgroup\$ Mar 27, 2021 at 17:54
  • 1
    \$\begingroup\$ I think you misunderstand that. This is systems that know about likely/unlikely but don't implement it will expand to white space. Implementations that don't understand it don't automatically do something special to support it. I could be wrong (does the language standard say something special)? \$\endgroup\$ Mar 27, 2021 at 17:58
  • 1
    \$\begingroup\$ It looks good to me, but @MartinYork is right, portability is not guaranteed, I ran it in a Windows environment both with gcc version 9.2.0 (tdm64-1) POSIX adapted version, and with MSVC and the compilation fails for both with undefined reference to __glibc_unlikely' and unresolved external symbol __glibc_unlikely respectively. \$\endgroup\$
    – anastaciu
    Mar 27, 2021 at 18:07
  • 1
    \$\begingroup\$ long is not size_t. long on wibdows has exact same representation as int. \$\endgroup\$ Mar 28, 2021 at 6:00

1 Answer 1

3
\$\begingroup\$
  • The function does not handle end-of-file correctly. stdin is just a stream, and could be closed like any other stream. For example, type Ctrl-D. Or

      echo -n a | ./a.out
      a????????
    

    Keep in mind that reading beyond end-of-file doesn't set errno.

  • There is no reason to special-case count == 0.

    An IO error doesn't break the loop. If it happened, all the subsequent invocations of fgets would return EOF, and upon the loop termination the count will be n.

  • Pass the buffer size as size_t. This is what sizeof yields, and the buffers could be large enough for their size to overflow an integer.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks a lot for your insight, please have a look at version 2 :) \$\endgroup\$ Mar 28, 2021 at 1:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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