11
\$\begingroup\$

I have implemented a C library to perform basic input operations on built-in data types with various error checks of more complexity. Any improvements or suggestions are appreciated.

GitHub

smartinput.h

/*
 * This library 'smartinput' is written in c programming.
 * by @venudayyam (binary_10)
 *  
 * The library contains subroutines which perform basic input
 * operations on a built-in data types and provide robust
 * error checking.
 * 
 * It is recommend not to mix the stadard input functions with these
 * library functions when performing operations on standard input stream
 * 
 * routines are meant to be used only when input stream is connected to keyboard.
 * routines expect valid arguments to be passed (if any).
 * 
 * 
 * 
 * 
 *  PROS:
 * 
 * - data overflow and underflow check on input
 * 
 * - input buffer is flushed(empty) after each operation.
 * 
 * - input value is said to be valid only when
 *   it can be stored in the specified variable (data type) `as it is`.
 *   otherwise it is invalid.
 * 
 */

#ifndef SMARTINPUT_H

#define SMARTINPUT_H
#include <stdbool.h>


bool get_char(char *ch);
char* get_line(char *_s, int _len);

/* trailing white spaces are preserved */
char* get_string(char *_s, int _len);  

bool get_short(short *value);
bool get_ushort(unsigned short *value);

bool get_int(int *value);
bool get_uint(unsigned int *value);

bool get_long(long *value);
bool get_ulong(unsigned long *value);

bool get_ullong(unsigned long long *value);
bool get_llong(long long *value);

bool get_float(float *value);
bool get_double(double *value);
bool get_ldouble(long double *value);


#endif

smartinput.c

/*
 * This library 'smartinput' is written in c programming.
 * by @venudayyam (binary_10)
 *  
 * The library contains subroutines to perform basic input
 * operations on a built-in data types and provide robust
 * error checking.
 * 
 * It is recommend not to mix the stadard input functions with these
 * library functions when performing operations on standard input stream
 * 
 * routines are meant to be used only when input stream is connected to keyboard.
 * routines expect valid arguments to be passed (if any).
 * 
 * 
 * 
 * 
 *  PROS:
 * 
 * - data overflow and underflow check on input
 * 
 * - input buffer is flushed(empty) after each operation.
 * 
 * - input value is said to be valid only when
 *   it can be stored in the specified variable (data type) `as it is`.
 *   otherwise it is invalid.
 * 
 */


#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <limits.h>
#include <errno.h>
#include "smartinput.h"

#define BUF_SIZE (1024)
static char buffer[BUF_SIZE];


char* get_line(char *_s, int _len)
{
    int ch;
    size_t i = 0;

    scanf(" ");
    while ((ch=getc(stdin)) != EOF && ch != '\n')
    {
        if (i < _len-1)
            _s[i++] = ch;
    }
    _s[i] = '\0';

    return _s;
}


char* get_string(char *_s, int _len)
{
    int ch;
    size_t i = 0;

    while ((ch=getc(stdin)) != EOF && ch != '\n')
    {
        if (i < _len-1)
            _s[i++] = ch;
    }
    _s[i] = '\0';

    return _s;
}


bool get_char(char *value)
{
    fgets(buffer,3,stdin);
    *value = buffer[0];
    if ((buffer[1] == '\0' && buffer[0] == '\n') ||
        (buffer[2] == '\0' && buffer[1] == '\n'))
    {
         return true;
    }
    else
    {
        if (buffer[0] != '\0')
        {
            scanf("%*[^\n]");
            getchar();
        }
        return false;
    }
}


bool get_short(short *value)
{
    char *temp;
    long v;

    get_line(buffer, BUF_SIZE);
    errno = 0;
    v = strtol(buffer,&temp,10);
    *value = v;
    if (errno == ERANGE || v > SHRT_MAX || v < SHRT_MIN 
                                        || *temp != '\0')
        return false;
    else
        return true;
}


bool get_ushort(unsigned short *value)
{
    char *temp;
    unsigned long v;

    get_line(buffer, BUF_SIZE);
    errno = 0;
    v = strtoul(buffer,&temp,10);
    *value = v;
    if (errno == ERANGE || v > USHRT_MAX || *temp != '\0')
        return false;
    else
        return true;
}


bool get_int(int *value)
{
    char *temp;
    long v;

    get_line(buffer, BUF_SIZE);
    errno = 0;
    v = strtol(buffer,&temp,10);
    *value = v;
    if (errno == ERANGE || v > INT_MAX || v < INT_MIN 
                                       || *temp != '\0')
        return false;
    else
        return true;
}


bool get_uint(unsigned int *value)
{
    char *temp;
    unsigned long v;

    get_line(buffer, BUF_SIZE);
    errno = 0;
    v = strtoul(buffer,&temp,10);
    *value = v;
    if (errno == ERANGE || v > UINT_MAX || *temp != '\0')
        return false;
    else
        return true;
}


bool get_long(long *value)
{
    char *temp;

    get_line(buffer, BUF_SIZE);
    errno = 0;
    *value = strtol(buffer,&temp,10);
    if (errno == ERANGE || *temp != '\0')
        return false;
    else
        return true;
}


bool get_ulong(unsigned long *value)
{
    char *temp;

    get_line(buffer, BUF_SIZE);
    errno = 0;
    *value = strtoul(buffer,&temp,10);
    if (errno == ERANGE || *temp != '\0')
        return false;
    else
        return true;
}


bool get_llong(long long *value)
{
    char *temp;

    get_line(buffer, BUF_SIZE);
    errno = 0;
    *value = strtoll(buffer,&temp,10);
    if (errno == ERANGE || *temp != '\0')
        return false;
    else
        return true;
}


bool get_ullong(unsigned long long *value)
{
    char *temp;

    get_line(buffer, BUF_SIZE);
    errno = 0;
    *value = strtoull(buffer,&temp,10);
    if (errno == ERANGE || *temp != '\0')
        return false;
    else
        return true;
}


bool get_float(float *value)
{
    char *temp;

    get_line(buffer, BUF_SIZE);
    errno = 0;
    *value = strtof(buffer,&temp);
    if (errno == ERANGE || *temp != '\0')
        return false;
    else
        return true;
}

bool get_double(double *value)
{
    char *temp;

    get_line(buffer, BUF_SIZE);
    errno = 0;
    *value = strtod(buffer,&temp);
    if (errno == ERANGE || *temp != '\0')
        return false;
    else
        return true;
}

bool get_ldouble(long double *value)
{
    char *temp;

    get_line(buffer, BUF_SIZE);
    errno = 0;
    *value = strtold(buffer,&temp);
    if (errno == ERANGE || *temp != '\0')
        return false;
    else
        return true;
}

sample.c

#include <stdio.h>
#include <string.h>
#include <stdbool.h>
#include "smartinput.h"

#define MAX_NAME 256

typedef struct _record
{
    char name[MAX_NAME];
    size_t id;
    char section;
}record;


void get_record(record *r)
{
    printf("name    : ");
    while (!get_line(r->name,MAX_NAME))
        fprintf(stderr,"error: invalid name. try again..\n");

    printf("section : ");
    while (!(get_char(&r->section) && r->section >= 'A'
                                   && r->section <= 'E'))
        fprintf(stderr,"error: invalid section. (select 'A'-'E'). try again..\n");

    printf("id      : ");
    while (!get_ulong(&r->id))
        fprintf(stderr,"error: invalid id. try again..\n");    
}


int main(void)
{
    record r;
    get_record(&r);
    printf("name : %s\nsec  : %c\nid  : %lu",r.name,r.section,r.id);
}

.

/* sample inputs and outputs of `get_int()` (note. sizeof(int) = 4) */

"2147483647\n"    -- true  (INT_MAX)
"2147483648\n"    -- false 
"-2147483648\n"   -- true  (INT_MIN)
"-2147483649\n"   -- false
"25ds\n"          -- false
"asdjkljasdf\n"   -- false 
"25 256\n"        -- false
"+1\n"            -- true
"-1\n"            -- true
EOF               -- false
" 1\n"            -- true
\$\endgroup\$
  • \$\begingroup\$ Good to add sample input, yet since input is text, better to clearly indicate the text entered input including white space and newlines like "123\n", " 123\n", "123 x\n", "\n123\n". \$\endgroup\$ – chux - Reinstate Monica May 3 '17 at 12:54
  • 1
    \$\begingroup\$ Please be careful with edits after the question has been answer. See this help page codereview.stackexchange.com/help/someone-answers. \$\endgroup\$ – pacmaninbw May 3 '17 at 14:03
  • \$\begingroup\$ I have only edited description here, the code is left unchanged @pacmaninbw \$\endgroup\$ – Binary_10 May 3 '17 at 14:08
  • \$\begingroup\$ Identifiers beginning with an underscore are reserved for the implementation - you'll want to fix some of your parameter names and structure tags. \$\endgroup\$ – Toby Speight May 4 '17 at 9:46
6
\$\begingroup\$

Overall, a worth goal and a good coding attempt, yet many problems.

  1. Questionable functionality in get_line() and get_string(). On EOF, both return a pointer to the supplied char array so the calling code lacks EOF distinguishably. Functions are certainly a problem should a rare input error occur.

  2. Questionable functionality in get_line() as it can get more that 1 line with scanf(" "). I'd expect the function to get 1 line of input only.

    char* get_line(char *_s, int _len) {
        ....
        scanf(" ");
    
  3. Use of int for buffer size when size_t is the right-size for array sizes.

    //char* get_line(char *_s, int _len);
    //char* get_string(char *_s, int _len); 
    char* get_line(char *_s, size_t _len);
    char* get_string(char *_s, size_t _len); 
    
  4. Mis-leading comment /* trailing white spaces are preserved */. For both get_line() and char* get_string(), the trailing white spaces (aside from '\n') are preserved.

  5. There is no function to get a line that includes the potential '\n'. C library defines a line as including that.

    A text stream is an ordered sequence of characters composed into lines, each line consisting of zero or more characters plus a terminating new-line character. Whether the last line requires a terminating new-line character is implementation-defined. C11 7.21.2 1

  6. With smartinput.c, put #include "smartinput.h" first as that will help detect any missing #include <xxx.h> that are needed in smartinput.h.

    #include "smartinput.h"
    #include <stdio.h>
    #include <string.h>
    ...
    
  7. Not a fan of using a global char buffer[] especially where a short local array would do.

  8. Integer conversion problems. 1) return value of get_line() is not checked - not that it helps much given the above no EOF detect issue. Without detecting EOF, conversion may use the previous contents of global buffer, 2) "No conversion" is not detected. 3) On conversion outside short range, errno is not set. and the value is set to the end of the short range. Similar troubles with all integer conversion functions. Alternative

  9. The get FP routines also fail to detect no conversion. Those FP routines have a special implementation defined (ID) behavior concerning underflow not handled by OP's code. Noted as follows:

    If the result underflows (7.12.1), the functions return a value whose magnitude is no greater than the smallest normalized positive number in the return type; whether errno acquires the value ERANGE is implementation-defined. §7.22.1.3 10

  10. Questionable test code. As get_line(s,...) always return s, the following will only print if r->name == NULL

    while (!get_line(r->name,MAX_NAME))   
      fprintf(stderr,"error: invalid name. try again..\n");
    
  11. Design asymmetry. The get line/string return the value on the others return a bool flag indicating success. I'd expect all to return bool.


  1. Minor: () not needed with #define BUF_SIZE (1024)

  2. Pedantic: char* get_line(char *_s, int _len) writes before _s if _len <= 0. Better to detect and return that perform UB. If code did call get_line(char *_s, 0), then if (i < _len-1) would become f (i < SIZE_MAX) allowing buffer overruns.

Alternative get_line(), something like:

char *get_line(char * s, size_t n) {
  if (n == 0) {
    return NULL; // or TBD on how to handle pathological case.
  }
  n--;

  int ch;
  size_t i = 0;
  while (i < n && (ch = fgetc(stdin)) != EOF) {
    s[i++] = (char) ch;
    if (ch == '\n') break;
  }
  s[i] = '\0';

  // Consume rest of line
  while (ch != '\n' && ch != EOF) {
    ch = fgetc(stdin);
  }

  if (ch == EOF) {
    if (i == 0 || ferror(stdin)) {
      return NULL;
    }
  }
  return s;
}
\$\endgroup\$
  • \$\begingroup\$ I have traded no conversion with the value of temp (endptr). my implementation is such that *temp will never be equal to '\0' if no conversion occurs (considering get_line() is unchanged) \$\endgroup\$ – Binary_10 May 3 '17 at 4:47
  • \$\begingroup\$ scanf(" "); A sequence of white-space characters (space, tab, newline, etc.; see isspace(3)). This directive matches any amount of white space, including none, in the input. The rest of characters until '\n' are read by get_line(). \$\endgroup\$ – Binary_10 May 3 '17 at 5:06
  • 1
    \$\begingroup\$ now i understand that by the term line you meant "line consisting of zero or more characters plus a terminating new-line character" so that "\n25x\n" is not a single line anymore. Yeah, i should probably rename get_line() to get_input(), take care of possible EOF input and make it static. Implement a new version of get_line() function. \$\endgroup\$ – Binary_10 May 4 '17 at 5:05
5
\$\begingroup\$

You shouldn't restrict your library routines only to work with the stdin stream specifically.

It would be way more useful and flexibly applicable, if you introduce another parameter of type FILE* for all of your functions:

bool fget_char(FILE* stream, char *ch);
char* fget_line(FILE* stream, char *_s, int _len);

/* trailing white spaces are preserved */
char* fget_string(FILE* stream, char *_s, int _len);  

bool fget_short(FILE* stream, short *value);
bool fget_ushort(FILE* stream, unsigned short *value);
// ...

and provide convenience implementations for

bool get_char(char *ch) {
    return fget_char(stdin,ch);
}
char* get_line(char *_s, int _len) {
    return fget_line(stdin,_s,_len);
}

/* trailing white spaces are preserved */
char* get_string(char *_s, int _len) {
    return fget_string(stdin,_s,_len);
}

bool get_short(short *value) {
    return fget_short(stdin,value);
}
bool get_ushort(unsigned short *value) {
    return fget_ushort(stdin,value);
}
// ...

which just call the corresponding fget_xxx() functions passing stdin there.

\$\endgroup\$

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.