0

I am C beginner and I am trying to create a program that is working with user input. The problem is that when user inputs incorrect data, program ends in an infinite error loop (saying "Incorrect input data"). I tried fflush and clearing user input with empty string, but it just somehow doesnt work.

Main.c:

#include <stdio.h>
#include "nd.h"
#include "nsd.h"
int num1;
int num2;
int a;
//char *p;

int main()
{
    while(!feof(stdin))
    {
        if(scanf("%d %d",&num1,&num2)==2)
        {
            if ((nd(num1)==1) && (nd(num2)==1))
            {
                printf("%s\n", "prime");
            }
            else
            {
                a=nsd(num1, num2);
                printf("%d\n", a);
            }           
        }
        else
        {
            fprintf(stderr, "Incorrect input data...");
            //fflush(stdin);
            //scanf ("%s",p);
        }
    }
    printf("%s\n", "DONE");
    return 1;
}

nd.c:

#include "nd.h"

#include <math.h>
#include <stdlib.h>

int nd(int a) {
   // calculate greatest divisor of "a"
   // nd equals 1, if it is prime
   int ret;
   int min_sqrt;

   if (a == 1) {
        ret = 1;
    } else {
        min_sqrt = abs(a) / 2;
        for (ret=min_sqrt; ret>1; ret--) {
            if ((a % ret)==0) {
                break;
            }
        }
    }
   return ret;
}
ondrousn
  • 57
  • 5
  • You have two `}`, so the second one is closing the while loop. – NSimon Nov 21 '14 at 09:40
  • @NSimon I think the while loop is closed correctly, I've fixed the indentation. – Tom Fenech Nov 21 '14 at 09:44
  • Tried the program, no infinite loop here... if you test it on Linux, try Ctrl+D after the final input. Also make sure that the functions `nd` and `nsd` terminate. – Erich Kitzmueller Nov 21 '14 at 09:45
  • http://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong Check this. – billpcs Nov 21 '14 at 09:47
  • 1
    Uncomment `scanf("%s",p);` and change `char *p;` to something like `char p[100];` so this call to `scanf()` actually has somewhere to put the results. [Then read this.](http://c-faq.com/stdio/scanfprobs.html) – r3mainer Nov 21 '14 at 09:54
  • which of the two parts is causing the infinite loop? it's really simple to figure it out. being a beginner is not an excuse. – Karoly Horvath Nov 21 '14 at 09:59
  • squeamish ossifrage - Please add it as an answer, problem solved. – ondrousn Nov 21 '14 at 09:59
  • regarding this line: while(!feof(stdin)) The feof function is not set to a valid value until after an input is made, so the first pass through the loop with working with an undefined value (its' initial value is implementation dependant. Also, if stdin is comming from user input at a terminal, then feof is never set, unless the user enters so should never be part of processing user input – user3629249 Nov 21 '14 at 16:30
  • this line: return 1; is indicating an error to the OS. the normal 'good/success' return value is 0 – user3629249 Nov 21 '14 at 16:34
  • before this line: if(scanf("%d %d",&num1,&num2)==2) there should be a printf indicating what the user is expected to input. otherwise, a user can run the program and be staring at a blank screen (and most of the user's tentative inputs will result in the error message about incorrect inputs.) This will result in the user tossing the program, rather than using it. I'd also suggest, at the top of main, printf describing the purpose of the program – user3629249 Nov 21 '14 at 16:48
  • why the requirement for two(2) input? this requirement seems to have no intrinsic value within this program. – user3629249 Nov 21 '14 at 16:51
  • regarding this line: if(scanf("%d %d",&num1,&num2)==2) there should be a leading ' ' in the format string, to enable skipping of white space (like '\n') – user3629249 Nov 21 '14 at 16:53

1 Answers1

1

This problem is occurring because the call to scanf("%d %d",...) will only match two integer numbers separated by white space. Anything else will be left in the input buffer. If your program loops around without clearing the buffer, the same thing will happen again.

You can fix this problem by uncommenting scanf("%s",p); and changing char *p; to something like char p[100]; so this call to scanf() actually has somewhere to put the results.

However, this isn't an ideal solution. The format string %s is inherently unsafe in calls to scanf() because it will match a sequence of non-whitespace characters of any length. That means your program is vulnerable to buffer overflows no matter how large you make the p[] buffer.

You can of course fix this by limiting the input length with a format string like %99s. But when handling interactive user input, it is often better to take a different approach, like this for example:

#include <stdlib.h>
char input[100], *end1, *end2;
int num1, num2, accept;

  :

while (1) {
  scanf("%99s",input);
  num1 = strtol(input,&end1,10);
  scanf("%99s",input);
  num2 = strtol(input,&end2,10);
  if (*end1 || *end2) puts("Illegal input :( Try again");
  else break;
}

Also, read this item in the comp.lang.c FAQ.

r3mainer
  • 23,981
  • 3
  • 51
  • 88
  • Hah, that'll teach me to code without testing. `if (!*end1 || !*end2)` should be `if (*end1 || *end2)`. Fixed now. – r3mainer Nov 21 '14 at 10:50