While you have found that your incorrect use of format specifiers and the need for a pointer to the variable in scanf was your primary problem, there is another aspect to using scanf properly not yet addressed.
You cannot simply write scanf ("%d", &intr_class); to take user input without validating the return to insure there was a successful conversion to integer that took place. You must do that before you can have any confidence that intr_class holds a valid value at all. (you will further want to insure that the value is within an acceptable range if you have any such requirement, before you attempt to use intr_class)
That goes for all user input regardless what function you use to take it, but it especially applies to scanf. What if in response to your prompt the user enters something wrong?, e.g.
Enter the number of instruction classes: rabbits
What then? Unless you check the return, you will never know intr_class is unset and if uninitialized, any attempt to access the value is Undefined Behavior.
Further, scanf stops reading when it encounters an error leaving any characters not read still in stdin. So what happens when you try and read something else? Let's think about an example:
Enter the frequency of the machine (MHz): 800
Does this entry succeed or fail? (answer: FAIL) Why? stdin still contains "rabbits\n800\n" (the '\n' is added every time the user presses Enter, and the only reason you don't fail every input is the '%d' format specifier consumes leading whitespace), but "rabbits" ain't whitespace...)
So you must be doubly careful using scanf for user input to account for not only whether the conversion succeeds or fails, but also for any characters left in the input buffer (either the '\n' if your next type of input does not consume leading whitespace, or any characters left in the input buffer after a failed conversion).
One way you manage this is to manually remove any characters that remain in stdin after each call to scanf. (note: should should check that the return is not EOF as well). After you call to scanf and check that the return is not EOF, you can simply call a simple function that reads and discards all remaining characters, like:
/* function to empty remaining characters in stdin before next input
* to help avoid the pitfalls associated with using scanf for user input.
*/
void emptystdin()
{
int c = getchar();
for (; c != '\n' && c != EOF; c = getchar()) {}
}
Avoiding the use of global variables and the very good suggestion to use a struct to hold all parameters should also be addressed. If you define a struct (call it struct parameters_t), create and declare an instance of it in main(), (say one called parameters), then all you need to do is pass the address of parameters to your params() function and fill the values in params. You simply change the function declaration for to include a pointer to the struct type, void params (stuct parameters_t *p) and then prompt and fill p->intr_class, etc. in the params function.
To make things simple (and avoid having to type struct parameters_t for the type all the time, you can simply create a typedef (an alias) defining parameters_t as an alias for struct parameters_t to cut down on typing.
You should also change the return type from void to something that can meaningfully indicate success/failure of your input in params
Putting those pieces together in a short example, you could do something similar to the following:
#include <stdio.h>
#include <stdlib.h>
/* declare a struct to associate all the values */
typedef struct {
int intr_class,
freq,
class1,
cpi_1,
class2,
cpi_2,
class3,
cpi_3;
} parameters_t;
/* function to empty remaining characters in stdin before next input
* to help avoid the pitfalls associated with using scanf for user input.
*/
void emptystdin()
{
int c = getchar();
for (; c != '\n' && c != EOF; c = getchar()) {}
}
/* params takes pointer to (address of) a struct parameters_t and
* prompts for and fills each value. a tmp struct is used to avoid
* changing any values in 'p' in case of a partial fill. returns
* address of p on success, NULL otherwise indicating error.
*/
parameters_t *params (parameters_t *p){
parameters_t tmp = { .intr_class = 0 };
printf ("Enter the number of instruction classes: ");
if (scanf ("%d", &tmp.intr_class) != 1)
return NULL;
printf ("Enter the frequency of the machine (MHz): ");
if (scanf ("%d", &tmp.freq) != 1)
return NULL;
printf ("Enter CPI of class 1: ");
if (scanf ("%d", &tmp.cpi_1) != 1)
return NULL;
printf ("Enter instruction count of class 1 (millions): ");
if (scanf ("%d", &tmp.class1) != 1)
return NULL;
printf ("Enter CPI of class 2: ");
if (scanf ("%d", &tmp.cpi_2) != 1)
return NULL;
printf ("Enter instruction count of class 2 (millions): ");
if (scanf ("%d", &tmp.class2) != 1)
return NULL;
printf ("Enter CPI of class 3: ");
if (scanf ("%d", &tmp.cpi_3) != 1)
return NULL;
printf ("Enter instruction count of class 3 (millions): ");
if (scanf ("%d", &tmp.class3) != 1)
return NULL;
*p = tmp; /* assign temp values to struct p */
return p;
}
/* simple function to print values stored in p */
void prnparams (parameters_t *p)
{
if (!p || p->intr_class == 0) {
fprintf (stderr, "error: parameters empty or NULL\n");
return;
}
printf ("parameters:\n"
" instruction classes: %d\n"
" frequency (MHz) : %d\n"
" CPI of class 1 : %d\n"
" class 1 inst count : %d\n"
" CPI of class 2 : %d\n"
" class 2 inst count : %d\n"
" CPI of class 3 : %d\n"
" class 3 inst count : %d\n",
p->intr_class, p->freq, p->cpi_1, p->class1,
p->cpi_2, p->class2, p->cpi_3, p->class3);
}
int main (void) {
char menuchoice;
/* declare a struct & iniailize all values zero */
parameters_t parameters = { .intr_class = 0 };
for (;;) { /* loop until user quits */
/* print out menu list */
printf ("\nMenu of Options:\n"
"______________\n"
" a) Enter Parameters\n"
" b) Calculate average CPI of a sequence of instructions\n"
" c) Calculate total execution time of a sequence of "
"instructions\n"
" d) Calculate MIPS of a sequence of instructions\n"
" p) Print stored values\n"
" e) Quit\n\n"
"Enter selection: ");
if (scanf ("%c", &menuchoice) == EOF) { /* check user cancels input */
putchar ('\n'); /* tidy up before exit */
break;
}
if (menuchoice != '\n') /* make sure user didn't just hit [Enter] */
emptystdin(); /* remove all chars from stdin */
switch(menuchoice){
case 'a':
if (!params(¶meters))
fprintf (stderr, "error: params() failed.\n");
emptystdin(); /* critical here or menuchoice would be '\n' */
break;
case 'b':
//avgCPI();
break;
case 'c':
//calcExTime();
break;
case 'd':
//calcMIPS();
break;
case 'p':
prnparams (¶meters);
break;
case 'e':
exit(0);
default:
fprintf (stderr, "error: invalid menuchoice.\n");
break;
}
}
return 0;
}
Example Use/Output
$ ./bin/scanfparams
Menu of Options:
______________
a) Enter Parameters
b) Calculate average CPI of a sequence of instructions
c) Calculate total execution time of a sequence of instructions
d) Calculate MIPS of a sequence of instructions
p) Print stored values
e) Quit
Enter selection: a
Enter the number of instruction classes: 10
Enter the frequency of the machine (MHz): 20
Enter CPI of class 1: 30
Enter instruction count of class 1 (millions): 40
Enter CPI of class 2: 50
Enter instruction count of class 2 (millions): 60
Enter CPI of class 3: 70
Enter instruction count of class 3 (millions): 80
Menu of Options:
______________
a) Enter Parameters
b) Calculate average CPI of a sequence of instructions
c) Calculate total execution time of a sequence of instructions
d) Calculate MIPS of a sequence of instructions
p) Print stored values
e) Quit
Enter selection: p
parameters:
instruction classes: 10
frequency (MHz) : 20
CPI of class 1 : 30
class 1 inst count : 40
CPI of class 2 : 50
class 2 inst count : 60
CPI of class 3 : 70
class 3 inst count : 80
Menu of Options:
______________
a) Enter Parameters
b) Calculate average CPI of a sequence of instructions
c) Calculate total execution time of a sequence of instructions
d) Calculate MIPS of a sequence of instructions
p) Print stored values
e) Quit
Enter selection: e
Look tings over. There is a lot to digest when you are just learning. So drop a comment if you have any questions and I'm happy to help further.
