Let's take a look at the problem one by one:
newline Remains in stdin after No. or Persons Read
printf("
Enter name: ");
safer_gets(person[i].full_name, 35); /* This is the step being skipped */
It is skipped because your safer_gets() reads only to the first '
' (newline character -- not a carriage-return, that is ' '). However, the first character saver_gets() sees in the input stream is the '
' character that remains in stdin unread after your call to scanf in:
printf("How many people do you want to generate labels for (0-10)? ");
scanf("%i", &x);
All scanf format specifiers for numeric conversion read only through the last digit (or decimal point) that makes up a number leaving the '
' generated by the user pressing Enter unread in the input-stream (stdin here). This is one of the primary reasons new C programmers are encouraged to read user input with a line-oriented input function such as fgets() (or POSIX getline()) and then use sscanf to parse values from the filled buffer.
Why Line-Oriented Input Functions are Preferred for User-Input
By using a line-oriented input function with sufficient buffer, the complete line of user-input is consumed (including the '
' from the user pressing Enter). This ensures that stdin is ready for the next input and doesn't have unread characters left-over from a prior input waiting to bite you.
Using All Input Functions Correctly
If you take nothing else from this answer, learn this -- you cannot use any input function correctly unless you check the return. This is especially true for the scanf family of functions. Why? If you are attempting to read an integer with scanf and the user enters "four" instead, then a matching failure occurs and character extraction from your input stream ceases with the first invalid character leaving all offending characters in your input stream unread. (just waiting to bite you again).
Using scanf Correctly
scanf can be used, if used correctly. This means you are responsible for checking the return of scanf every time. You must handle three conditions
(return == EOF) the user canceled input by generating a manual EOF by pressing Ctrl+d (or on windows Ctrl+z);(return < expected No. of conversions) a matching or input failure occurred. For a matching failure you must account for every character left in your input buffer. (scan forward in the input buffer reading and discarding characters until a '
' or EOF is found); and finally(return == expected No. of conversions) indicating a successful read -- it is then up to you to check whether the input meets any additional criteria (e.g. positive integer, positive floating-point, within a needed range, etc..).
You also must account for what is left in your input stream after a successful read with scanf. As discussed above, scanf will leave the '
' in the input stream unread for ALL conversion specifiers unless you specifically account for it in your format string (which, if accounted for, usually results in a fragile input format string, easily foiled by additional extraneous characters after the wanted input but before the '
') When using scanf for input, you must put your accountant's hat on and account for every character that remains in the input stream and empty the input stream of any offending characters when required.
You can write a simple empty_stdin() function to handle removing all extraneous characters that remain after a user input by simply scanning forward discarding all characters that remain until the '
' is found or EOF encountered. You do that to a varying degree in your safer_gets() function. You can write a simple function as:
void empty_stdin(void)
{
int c = getchar(); /* read character */
while (c != '
' && c != EOF) /* if not '
' and not EOF */
c = getchar(); /* repeat */
}
You can do the same thing with a simple for loop inline, e.g.
for (int c = getchar(); c != '
' && c != EOF; c = getchar()) {}
Next Problem -- Attempting to Write to Invalid Address
When using scanf, scanf expects the parameter for a corresponding conversion to be a pointer to the appropriate type. In:
printf("
Enter zipcode: ");
scanf("%ld", person[i].zip_code); /* I get a bogus number here */
You fail to provide a pointer, providing a long int value instead. Since person[i].zip_code is type long int in order to provide a pointer for scanf to fill you must use the address-of operator, e.g. &person[i].zip_code to tell scanf which address to fill with the value it provides a conversion for.
Wait? Why don't I have to do that with array? On access, an array is converted to a pointer to the first element. So for string input, if an array is being used to hold the string, it is automatically converted to a pointer C11 Standard - 6.3.2.1 Other Operands - Lvalues, arrays, and function designators(p3).
toupper Operates on Characters not Strings
printf("%s
", toupper(person[i].state)); /* This is where the error code is occurring */
As discussed in my comment, toupper takes type int as the parameter, not type char*. To convert a string to upper/lower case, you need to loop over each character converting each character individually. However, in your case with the .state member of your struct, there are only 2 characters to worry about, so just convert them both when they are read, e.g.
/* just 2-chars to convert to upper - do it here */
person[i].state[0] = toupper (person[i].state[0]);
person[i].state[1] = toupper (person[i].state[1]);
Fundamental Problems in safer_gets()
That takes care of the most of the obvious problems, but the safer_gets() function itself has several fundamental problems. Specifically, it fails to handle EOF when returned by getchar() and it fails to provide any indication to the user whether the requested user-input succeeded or failed due to returning nothing with type void. In any function you write, where there is any possibility of failure within the function, you MUST provide a meaningful return to the calling function to indicate whether the requested operation of the function succeeded or failed.
What can you do with safer_gets()? Why not return a simple int value providing the number of characters read on success, or -1 (the normal value for EOF) on failure. You get the double-bonus of now being able to validate whether the input succeeded -- and you also get the number of character in the string (limited to 2147483647 chars). You also now have the ability to handle a user canceling the input by generating a manual EOF with Ctrl+d on Linux or Ctrl+z (windows).
You should also empty stdin of all characters input in ALL cases other than EOF. This ensures there are no characters that remain unread after your call to safer_gets() that can bite you if you make a call to another input function later. Making those changes, you could write your safer_gets() as:
/* always provide a meaninful return to indicate success/failure */
int safer_gets (char *array, int max_chars)
{
int c = 0, nchar = 0;
/* loop while room in array and char read isn't '
' or EOF */
while (nchar + 1 < max_chars && (c = getchar()) != '
' && c != EOF)
array[nchar++] = c; /* assing to array, increment index */
array[nchar] = 0; /* nul-terminate array on loop exit */
while (c != EOF && c != '
') /* read/discard until newline or EOF */
c = getchar();
/* if c == EOF and no chars read, return -1, otherwise no. of chars */
return c == EOF && !nchar ? -1 : nchar;
}
(note: above the test on nchar + 1 < max_chars ensures that a character remains for the nul-terminating character, and is just a safer rearrangement of nchar < max_chars - 1)
General Approach to Input Validation
Now, you have an input function you can use that indicates success/failure of input allowing you to validate the input back in the calling function (main() here). Take for example reading the .full_name member using safer_gets(). You can't just blindly call safer_gets() and not know whether input was canceled or a premature EOF encountered and use then proceed to use the string it filled with any confidence in your code. *Validate, validate, validate each expression. Back in main(), you can do that by calling safer_gets() as follows to read .full_name (and every other string variable):
#define NAMELEN 35 /* if you need a constant, #define one (or more) */
#define ADDRLEN 50 /* (don't skimp on buffer size) */
...
for (;;) { /* loop continually until valid name input */
fputs ("
Enter name : ", stdout); /* prompt */
int rtn = safer_gets(person[i].full_name, NAMELEN); /* read name */
if (rtn == -1) { /* user canceled input */
puts ("(user canceled input)");
return 1; /* handle with graceful exit */
}
else if (rtn == 0) { /* if name empty - handle error */
fputs (" error: full_name empty.
", stderr);
continue; /* try again */
}
else /* good input */
break;
}
(note: the return of safer_gets() is captured in the variable rtn and then evaluated for -1 (EOF), 0 empty-string, or greater than 0, good input)
You can do that for each string variable you need to use, and then use the same principals discussed above to read and validate .zip_code. Putting it altogether in a short example, you could
