There are a large number of small problems in your code. The most critical are the attempts to use ==
for string-equality instead of strcmp()
. The next being the inclusion of '&'
before the array name when using scanf()
. Upon access, an array is converted to a pointer to its first element, so there is no need for '&'
before the array name in scanf()
after "%s"
. See C11 Standard - 6.3.2.1 Other Operands - Lvalues, arrays, and function designators(p3)
Another issue is you must protect your array bounds when using the "%s"
conversion specifier. It cannot be used safely otherwise. Why? "%s"
will write as many non-whitespace characters as it is provided to the address given in the argument list. If given 500 bytes of shell-code, it will happily write all 500 bytes corrupting your stack everywhere "%s"
is used with scanf()
. (especially with answer
which you declare as only 4-bytes long -- which is horribly insufficient) LESSON - Never skimp on buffer size!
To use the field-width modifier with scanf()
when using "%s"
you add the maximum number of bytes to read (not including the nul-terminating character), e.g. "199s"
when writing to name
. This ensures scanf()
will not attempt to write more than 199
characters to name (plus the nul-terminating character)
All of this (and a lot more) are the pitfalls of scanf()
and are why it is recommended to take all user input using fgets()
or POSIX getline()
. Provided a sufficiently sized buffer, the line-oriented input functions will consume an entire line of data. This eliminates all the problems inherent with scanf()
and the only thing you need do is to trim the trailing '\n'
read and included in the buffer they fill. This is trivial using strcspn()
. It simply counts the number of initial characters that are not in its reject string -- providing an easy way to know how long the string is up to the '\n'
. A better way to prompt, read/VALIDATE and trim the '\n'
is:
fputs ("What are you looking for?: ", stdout); /* no conversion */
/* read/VALIDATE all input with fgets..
* it will consume a line at-a-time
* (given sufficient storage)
*/
if (!fgets (name, MAXNM, stdin)) {
return 0;
}
name[strcspn (name, "\n")] = 0; /* trim '\n' from end of name */
The other issue you have is using fflush(stdin);
. The way you are using it is Undefined according to the C standard. fflush
is not defined for input streams, see: C11 Standard - 7.21.5.2 The fflush function(p2) and fflush(stdin); leads to undefined behaviour on most systems.. It is not portable. For example, on Linux, fflush
is only valid on seekable streams. Which only applies to stdin
if a file is redirected as input (e.g. ./prog <somefile
). See: fflush(3) - Linux manual page. Full discussion Standard C and POSIX leave fflush(stdin) as undefined behaviour
(MS is the only compiler I know of that provides a Non-Standard implementation that does clear stdin
the way you are using it. -- Don't do that, it is virtually 100% non-portable to any other compiler)
I won't go over all the points detailed in the other good answers. But one additional note as you are learning to program. Always compile with warnings enabled, and do not accept code until it compiles without warning. To enable warnings add -Wall -Wextra -pedantic
to your gcc/clang
compile string (also consider adding -Wshadow
to warn on shadowed variables). You can add -Werror
to have all warnings treates as errors (recommended) For VS (cl.exe
on windows), use /W3
. All other compilers will have similar options. Read and understand each warning -- then go fix it. The warnings will identify any problems, and the exact line on which they occur. You can learn a lot by listening to what your compiler is telling you.
To put things altogether for you what I've done is just to change your implementation to use fgets()
instead of scanf()
while leaving your code commented out below so you can easily see exactly what changes are necessary. I have commented the code heavily so you can understand why the changes were made. I also changed the return type from void
to int
so you can provide a meaningful return indicating whether your function succeeded or failed. You will see how that benefits your code in main()
.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define MAXINM 50 /* if you need a constant, #define one (or more) */
#define MAXNM 200
struct category {
char item_name[MAXINM];
int item_quantity;
float item_price;
};
/* choose a meaningful return type that can indicate whether
* the function succeeded or failed. (0 - not found, 1 - found)
*/
int items_search (struct category a[], int length)
{
char answer[MAXNM] = "yes", /* a string-literal initializer is fine */
name[MAXNM] = "",
tmp[MAXNM]; /* temporary array for input use */
int i = 0, /* group declarations by type */
// item_quantity = 0, /* UNUSED VARIABLE */
number = 0,
transaction = 0; /* flag indicating success/failure */
fputs ("What are you looking for?: ", stdout); /* no conversion */
/* read/VALIDATE all input with fgets..
* it will consume a line at-a-time
* (given sufficient storage)
*/
if (!fgets (name, MAXNM, stdin)) {
return 0;
}
// scanf ("%s", &name); /* scanf is full of pitfalls */
/* no & before name, it's a pointer */
name[strcspn (name, "\n")] = 0; /* trim '\n' from end of name */
for (; i < length; i++) { /* loop over each item in struct */
if (strcmp (a[i].item_name, name) == 0) { /* strcmp for equality */
fputs ("Desired number of the item?: ", stdout);
/* read/VALIDATE using fgets() */
if (!fgets (tmp, MAXNM, stdin)) {
return 0;
}
/* minimum validation of conversion with sscanf,
* for full error reporting, use strtol()
*/
if (sscanf (tmp, "%d", &number) != 1) {
fputs ("error: invalid integer input.\n", stderr);
return 0;
}
// scanf ("d", &number);
// fflush (stdin); /* defined for "seekable" streams */
if (number <= a[i].item_quantity) { /* less than or equal */
printf ("Order is possible.\n"
"Total Price: $%.2lf\n"
"Should the order be placed?: ",
number * a[i].item_price);
/* read/VALIDATE using fgets() */
if (!fgets (tmp, MAXNM, stdin)) {
return 0;
}
// scanf ("%s", &answer);
// fflush (stdin);
if (strcmp (answer, "yes") == 0) {
puts ("The order is placed");
}
else {
puts ("Order was cancelled");
}
}
else {
printf ("There are only %d pieces of the item.\nShould the "
"order be canceled?: ", a[i].item_quantity);
/* read/VALIDATE using fgets() */
if (!fgets (tmp, MAXNM, stdin)) {
return 0;
}
answer[strcspn (answer, "\n")] = 0; /* trim '\n' */
// scanf ("%s", &answer); /* answer is already a pointer */
// fflush (stdin);
if (strcmp (answer, "yes") == 0) { /* strcmp for equality */
puts ("Order was cancelled. Should be bought later?");
/* read/VALIDATE using fgets() */
if (!fgets (tmp, MAXNM, stdin)) {
return 0;
}
answer[strcspn (answer, "\n")] = 0; /* trim '\n' */
// scanf ("%s", &answer);
// fflush (stdin);
if (strcmp (answer, "yes") == 0) { /* strcmp */
puts ("\nThe order for later is placed");
}
else {
puts ("\nThe order was cancelled");
}
}
else {
puts ("The order is placed");
}
}
transaction = 1; /* set transaction complete flag */
}
// else { /* not needed here */
// printf ("The specified item does not exist\n");
// }
}
return transaction; /* return transaction status */
}
int main (void)
{
struct category laptops[] = {
{ "Asus_365", 7, 499.00 },
{ "Lenovo_L49", 30, 699.91 },
{ "HP_Alien", 20, 649.99 },
{ "Acer Alpha touch", 10, 899.99 }
};
if (!items_search (laptops, sizeof laptops / sizeof *laptops)) {
fputs ("error: incomplete transaction.\n", stderr);
}
}
Example Use/Output
$ ./bin/transaction
What are you looking for?: HP_Alien
Desired number of the item?: 2
Order is possible.
Total Price: $1299.98
Should the order be placed?: yes
The order is placed
or
$ ./bin/transaction
What are you looking for?: HP_Gorilla
error: incomplete transaction.
Look things over and let me know if you have any further questions. NOTE: this isn't the only way to approach this, just one way that improves over the UNVALIDATED use of scanf()
and eliminates the potential for exploit through buffer overflow from the unlimited use of "%s"
.