2
\$\begingroup\$

As written [above], getint treats a + or - not followed by a digit as a valid representation of zero. Fix it to push such a character back on the input.

Please check this is right.

If I fully understand what have to be done, that's check whether the character after sign is a digit or not.

#include <stdio.h>
#include <ctype.h>


#define BUFSIZE 100
char buf[BUFSIZE];
int bufp = 0;

#define SIZE 1000

int getch(void);
void ungetch(int);


int main()
{
    int n,i, array[SIZE], getint(int *);

    for (n = 0;n < SIZE && getint(&array[n]) != EOF;n++)
        ;
    for(i = 0; i < n;i++)
    printf("i:%d array:%d\n",i,array[i]);
}

int getint(int * pn)
{
    int c, sign;

    while (isspace(c = getch()))
        ;

    if (!isdigit(c) && c != EOF && c != '+' && c != '-'){
        ungetch(c);
        if(!isdigit(getch())) 
          
            return 0;
        
    }
    sign = (c == '-') ? -1 : 1;
    if (c == '+' || c == '-')
        c = getch();
    for(*pn = 0; isdigit(c); c = getch())
        *pn = 10 * *pn + (c-'0');
    *pn *= sign;
    if (c != EOF)
        ungetch(c);
    return c;
}



int getch(void)
{
    return (bufp > 0) ? buf [--bufp] : getchar();
}
void ungetch(int c)
{
    if (bufp >= BUFSIZE)
        printf ("ungetch: too many characters\n");
    else
        buf[bufp++] = c;
}
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Please summarise what the code does (both in the description and the title). I've heard there are programmers who don't even own the K&R book. \$\endgroup\$ Commented Jul 4, 2023 at 12:04

3 Answers 3

2
\$\begingroup\$

Avoid Global Variables

It is very difficult to read, write, debug and maintain programs that use global variables. Global variables can be modified by any function within the program and therefore require each function to be examined before making changes in the code. In C and C++ global variables impact the namespace and they can cause linking errors if they are defined in multiple files. The answers in this Stack Overflow question provide a fuller explanation.

Declare the Variables as Needed

In the original version of C back in the 1970s and 1980s variables had to be declared at the top of the function. That is no longer the case, and a recommended programming practice is to declare the variable as needed. In C the language doesn't provide a default initialization of the variable so variables should be initialized as part of the declaration. For readability and maintainability each variable should be declared and initialized on its own line.

Code Organization

Function prototypes are very useful in large programs that contain multiple source files, and that in case they will be in header files. In a single file program like this it is better to put the main() function at the bottom of the file and all the functions that get used in the proper order above main(). Keep in mind that every line of code written is another line of code where a bug can crawl into the code.

Prefer Braces { and } Around Single Statements in if or loops

Some programmers consider this a style issue, but it makes it much easier to read and maintain the code if each in an if, else or loop block is embedded within braces. Extending the functionality of these statements can be problematic when the braces are not used. For a more in depth discussion of this see the first 2 answers on this Stack Overflow question. As one of the answers points out this is true in all C like languages (C, C++, C#, JavaScript, Java, etc.). I have worked at multiple companies where this was required in the coding standard and flagged during code reviews.

Prefer size_t for Index Variables

The size_t type did not exist at the time of the writing of the K & R C book, but it is a better type to used as an index into arrays and for many for loop control variables. The size_t type is the largest native unsigned integer the system will support.

int main(void)
{
    size_t n = 0;
    int array[SIZE];

    for (size_t n = 0; n < SIZE && getint(&array[n]) != EOF; n++)
        ;

    for (size_t i = 0; i < n; i++)
        printf("i:%zd array:%d\n", i, array[i]);
}

Use void in int main(void) declaration

When a program is supposed to process arguments, main() should be declared

int main(int argc, char* argv[])

When there will be no arguments to process main() should be declared as

int main(void)

This provides better self documenting code.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ C never required variables to be declared at the beginning of the function, only the containing block. I never did find out whether B had such a restriction, or whether this common misunderstanding is founded elsewhere. \$\endgroup\$ Commented Jul 5, 2023 at 10:36
1
\$\begingroup\$

Improve the testing

There's no need to store all the results in array. Just print them as they are generated. It's important to consider the return value from getint() (0, EOF, or positive), since this is what indicates whether the number was parsed.

int main(void)
{
    int n = 0;
    int value;
    int result;

    while ((result = getint(&value)) != EOF) {
        if (result) {
            printf("%d - value=%d\n", n, value);
        } else {
            printf("%d: not a number\n", n);
        }
        ++n;
    }
}

With this main(), the first test I tried produced incorrect results:

./285902 <<<+a5
0 - value=0
1: not a number
2 - value=5

We correctly identify that a is not a number, but still claim that + without a digit is a valid representation of zero.

The logic error is clear:

    if (!isdigit(c) && c != EOF && c != '+' && c != '-'){
        ungetch(c);
        if(!isdigit(getch())) 
          
            return 0;

We put c back in the input stream and then fetched it again using getch(). So this test is always true (because we already know it's a non-digit, due to the if (!isdigit(c) && …) preceding).

The test we need to make is just after this point, where we have the character immediately following the + or -:

    if (c == '+' || c == '-')
        c = getch();
\$\endgroup\$
1
\$\begingroup\$

Overflow when the string represents INT_MIN

Example: when INT_MIN == -2147483648:

*pn = 10 * *pn + (c-'0'); will incur int overflow, which is undefined behavior (UB) as it attempts to make 2147483648 from "-2147483648".

Consider alternatives.

Instead of accumulating the positive value, accumulate the negative:

//             v
*pn = 10 * *pn - (c-'0');

No overflow protect/detection/reporting

No detection of non-numeric input

No detection input error

Review error indicator.


Alternative that reads directly from stdin. Notice a good chunk of it is documentation.
(Demonstrative code - I'' test later)

#include <ctype.h>
#include <limits.h>
#include <stdbool.h>
#include <stdio.h>

/*
 * Read numeric text from _stdin_ and convert to an _int_.
 *
 * Leading white-spaces are read and discarded.
 * An optional +/- sign is read.
 * All digits are then read.
 * Any character read after the digits is put back into stdin.
 * When no digits read, the sign character, if any, is put back into stdin.***

 * The converted value is based on the sign and digits read as base 10.
 * The converted value is 0 if no numeric text read.
 * Out of `int` range conversions, overflow, result in `INT_MAX/INT_MIN`.
 * If pn != NULL, the converted value is saved in *pn.
 *
 * Return value:
 * Any time an input error occurs (rare),
 *   the return value is EOF and no characters are put back into stdin.
 * If no non-white-space characters read, the return value is EOF.
 * Otherwise if overflow or no numeric text read, the return value is 0.
 * Otherwise the return value is 1 (success).
 */
int getint(int *pn) {
  // Consume leading white-spaces.
  int ch;
  while (isspace((ch = getchar()))) {
    ;
  }

  // Record the sign, if any.
  bool sign_input = false;
  int sign = ch;
  if (ch == '-' || ch == '+') {
    ch = getchar();
    sign_input = true;
  }

  // Accumulate digits.
  bool overflow = false;
  bool digit_input = false;
  int sum = 0;
  while (isdigit(ch)) {
    digit_input = true;
    if (sum <= INT_MIN / 10
        && (sum < INT_MIN / 10 || ch > '0' - (INT_MIN % 10))) {
      sum = INT_MIN;
      overflow = true;
    } else {
      sum = sum * 10 - (ch - '0');
    }
    ch = getchar();
  }

  // Adjust for sign
  if (sign != '-') {
    if (sum < -INT_MAX) {
      sum = INT_MAX;
      overflow = true;
    } else {
      sum = -sum;
    }
  }

  // Save the conversion.
  if (pn) {
    *pn = sum;
  }

  if (ch == EOF) {
    // If no input read or an input error just occurred ...
    if (!(digit_input || sign_input) || !feof(stdin)) {
      return EOF;
    }
  } else {
    ungetc(ch, stdin);
  }

  /* Additionally if no digits were read,
   * yet a sign was read, we should unget the sign too.
   * If this is attempted or not should be documented as part
   * of the function.
   * ***
   * > One character of pushback is guaranteed.
   * > If the ungetc function is called too many times
   * > on the same stream without an intervening read
   * > or file positioning operation on that stream,
   * > the operation may fail. C23dr 7.23.7.10 3
   */
  if (!digit_input && sign_input) {
    ungetc(sign, stdin);
  }
  // Other code may want to test the return values of ungetc().

  // Return success if no overflow and something numeric was read.
  return !overflow && digit_input;
}
```
\$\endgroup\$

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.