1
\$\begingroup\$

I have written a function that reads standard input, ignores initial whitespace and fills the buffer with that data.
I think that it is a bit large and cumbersome, can you please tell me what can be improved?

int dscan(char *where, int size) {
    int found = 0;
    int i = 0;
    int j = 0;
    for (i = 0; i < size - 1; i++) {
        char c;
        scanf("%c", &c);
        if (c == '\n') {
            where[j] = '\n';
            where[j + 1] = '\0';
            return j;
        }
        if (isspace(c)) {
            if (!found) {
                continue;
            }
        } else {
            found = 1;
        }
        where[j] = c;
        j++;
    }
    return j;
}
\$\endgroup\$
2
\$\begingroup\$

You must check the return value of scanf. If you don't, you run into an endless loop when reaching the end of the file. And, even worse, you invoke undefined behavior.

You could split the loop into two: first, skip all whitespace, then read the remaining text.

int dscan(char *buf, int bufsize) {
    int ch;

    while ((ch = getc()) != EOF && ch != '\n' && isspace(ch)) {
        continue;
    }

    int idx = 0;
    while (ch != EOF && ch != '\n' && idx < bufsize) {
        buf[idx++] = ch;
        ch = getc();
    }

    if (ch == '\n' && idx < bufsize) {
        buf[idx++] = '\n';
    }
    if (idx < bufsize) {
        buf[idx++] = '\0';
    }

    return idx;
}

I changed several things:

  • EOF is handled correctly. On EOF, the string is returned, properly null-terminated. (This was not the case in your code.)
  • Leading whitespace does not count towards the buffer, it is simply ignored, like you said in your description. Your code counted it.
  • Each call to an I/O function is checked for an error result.
  • It's easy to strip the newline character from the string by removing the corresponding if sentence. Depending on whether the calling code needs this or not.
  • If dscan(buf, size) < size, the buffer is a string, i.e. it is properly null-terminated.
  • Calling isspace does not invoke undefined behavior. In your code, if I enter a negative char, the behavior is undefined. See the <ctype.h> documentation.
\$\endgroup\$
  • 1
    \$\begingroup\$ Goal is "scan input to string". With if (idx < bufsize), buf might not get a null character termination. Best to always null character terminate as long as bufsuize > 0. \$\endgroup\$ – chux - Reinstate Monica Oct 30 '17 at 14:42
  • 1
    \$\begingroup\$ "If dscan(buf, size) < size, the buffer is a string, i.e. it is properly null-terminated." is a problem - it is true, yet consider: bufsize == 5, Text entered is "123\n". Return value is 5 and buf is a valid string. With 5 not < 5, the return value gives a false indication of non-success. \$\endgroup\$ – chux - Reinstate Monica Oct 30 '17 at 14:49
  • 1
    \$\begingroup\$ When the buffer size is insufficient, this answer still performs a getc(); and so that character is lost to subsequent I/O. Better to only get when there is enough buffer space or un-get the character. \$\endgroup\$ – chux - Reinstate Monica Oct 30 '17 at 14:53
  • 1
    \$\begingroup\$ Minor: size_t bufsize is a type that can encode all array sizes. int bufsize may be insufficient (for large arrays). \$\endgroup\$ – chux - Reinstate Monica Oct 30 '17 at 14:59
2
\$\begingroup\$

what can be improved?

The contract "scan input to string buffer" and "a function that reads standard input, ignores initial whitespace and fills the buffer with that data." lacks directions on how to handle many cases.

  1. Code appears to be reading until end-of-line, yet that is not specified. Even if so, with ignoring leading white-space, it makes sense to also ignore trailing-white-space including the potential '\n'.

  2. Appending a '\0' to insure a string does meet the title requirement. Should anything special happen if a null character '\0' is read?

  3. Buffer size too small? What to do with extra characters? Consume them, leave them for the next I/O functions?

  4. How to handle an immediate end-of-file? Or an end-of-file after some characters?

  5. How to handle a rare input error?

  6. How to handle pathological input like where == NULL, size <= 0?

  7. Array sizes exceeding INT_MAX?

Perhaps some of these corner case result in a "don't care" or UB as the contract does not specify a behavior. Yet consider a production task with the loose requirements.

Customers expect code to behave well is "all" situations. Although every conceivable issue is a worthy goal and not truly attainable, as an implementor, code should be resilient to many situations. These include ones that are not considered by the customer, yet reasonably possible. For corner cases that significantly reduce performance, greater coding goal detail is needed.


@Roland Illig addresses some coding weakness in OP's code. Some additional ideas:

// Add functional comment
// Return 0 when no data read or error

// int dscan(char *where, int size) {
// Use size_t in and out to convey buffer size and usage
size_t dscan(char *where, size_t size) {

  // Assess input values
  if (where == NULL || size == 0) {
    return 0;
  }

  // decrement to save space for a \0
  size--;

  // Read the entire line
  // getchar() better than scanf("%c", &c)
  // It gets an unsigned char value and indicates end-of-file/error
  int c;
  size_t i = 0;
  size_t non_white_space_next = 0;
  while ((c = fgetc(stdin)) != EOF && c != '\n') {
    if (i < size) {
      if (isspace(c)) {
        if (i == 0) continue;
        where[i++] = c;
      } else {
        where[i++] = c;
        non_white_space_next = i;
      }
    } else {
      ; // What to do with too many characters?
      // For now, code will ignore them
      // Yet code should convey that buffer was too small.
    }
  }

  // If code is to ignore trailing space
  i = non_white_space_next;

  // Always append a null character
  where[i] = '\0';

  // Typically if no characters are read or an error occur,
  // code should return 0 to indicate
  if (c == EOF && (ferror(stdin) || i == 0)) {
    return 0;
  }
  return i+1;
}
\$\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.