0

I am coding a function to validate a number inserted by the user (between 1 and 3) and it doesn't matter what number I put the program crashes.

Here's my code:

    int validate(int low, int high) {
    int selection;

    do {
    scanf("%d", selection);
    if (selection < low || selection > high)
        printf("Invalid Input, try again: ");
    } while (selection > low || selection < high);

    return selection;
}

Anyone any idea, please?

Unheilig
  • 16,196
  • 193
  • 68
  • 98
  • 1
    [Never use `scanf`](https://stackoverflow.com/questions/24302160/scanf-on-an-istream-object/24318630#24318630). But if you insist, `&selection`, not `selection`. – zwol Oct 13 '15 at 23:40
  • consider adding error that you are getting – Marquis Blount Oct 14 '15 at 00:10
  • Add the crash message, including the trace-back. Also, note that your while condition is always true. Consider changing your if to "if (condition) printf ("Error") else flag = false; this lets you do the check only once and run the loop on a condition of "while flag". Alternately, run it on "while true" and use a "break" to exit. – Prune Oct 14 '15 at 00:16

2 Answers2

1

While reading scanf, use & with the variable 

 int validate(int low, int high) {
    int selection;
     printf("Enter your Selection ");
      do {
            scanf("%d", &selection);
            if (selection < low || selection > high)
           {
                printf("Invalid Input, try again: ");
          }
       } while (selection < low || selection > high) ;

        return selection;
    }
makdu
  • 908
  • 2
  • 13
  • 26
0

2 bugs I see.

  1. makdu, mention the scanf issue.

  2. your while loop should use the && operator.

    } while (selection > low && selection < high);

Ejaski
  • 199
  • 5
  • Is the while logic correct @Ejaski ? He wants to break the look when Selection has a value between low and high ! – makdu Oct 13 '15 at 23:58
  • Thank you all for your suggestions! But why shouldn't I use scanf? what else do you use? At school I've learnt only that one.. –  Oct 14 '15 at 00:11
  • scanf() and gets() both are susceptible to buffer overflow vulnerabilities. Meaning you don't specify how much memory you are passing to it, so the user could possibly send more information back which would corrupt memory. A better option is fgets(). – Ejaski Oct 14 '15 at 01:23
  • @Ejaski Yes, and also, numeric conversions in `scanf` trigger *undefined behavior* on overflow. For what the OP is trying to do, there probably won't be an issue, but still, `fgets` (or `getline`) + `strto(u)l` is the Proper Way. – zwol Oct 14 '15 at 19:37