I hope that someone can help me since I'm clueless about the problem in my code. I am a new starter in programming and a week ago I tried an exercise which I completely failed. I tried to compare the elements in the array with the input text on the compiler. But the ("if") is always giving a false. Even if I write down an existing element of the array on the compiler. I know that there must be a lot of logical mistakes but I am really helpless. I am really looking for your answers. Thanks for the help!
#include <stdio.h>
#include <stdlib.h>
struct category {
char item_name[50];
int item_quantity;
float item_price;
};
void items_search (struct category a[], int length)
{
char answer[] = { "yes" };
int number = 0;
int item_quantity = 0;
char name[200];
int i = 0;
printf ("What are you looking for?\n");
scanf ("%s", &name);
if (a[i].item_name == name) {
printf ("What is the desired number of the item?");
scanf ("d", &number);
fflush (stdin);
if (number == a[i].item_quantity) {
printf ("Order is possible. Should the order be placed?\n");
scanf ("%s", &answer);
fflush (stdin);
if (answer == "yes") {
puts ("The order is placed");
}
else {
puts ("Order was cancelled");
}
}
else {
printf ("There are only %d pieces of the item. Should the "
"order be canceled?\n", a[i].item_quantity);
scanf ("%s", &answer);
fflush (stdin);
if (answer == "yes") {
puts ("Order was cancelled. Should be bought later?");
scanf ("%s", &answer);
fflush (stdin);
if (answer == "yes") {
puts ("The order is placed");
}
else {
puts ("The order was cancelled");
}
}
else {
puts ("The order is placed");
}
}
}
else {
printf ("The specified item does not exist\n");
}
}
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}
};
items_search (laptops, sizeof (laptops) / sizeof (struct category));
}
CodePudding user response：
You should not compare a pointer to a string:
if(answer=="yes"){
Instead, you should use strncmp:
if(strncmp( answer, "yes", 3) == 0)
CodePudding user response：
For starters these calls of scanf
scanf ("%s", &name);
and
scanf ("%s", &answer);
are incorrect. You should write
scanf ("%s", name);
and
scanf ("%s", answer);
Arrays do not have the comparison operator. So in these if statements
if (a[i].item_name == name) {
and
if (answer == "yes") {
there are compared two pointers to first elements of two different arrays (due to the implicit conversion of array designators to pointers to their first elements) that occupy different extents of memory. So their comparison will always evaluate to logical false.
Instead you need to use the C string function strcmp like
if ( strcmp( a[i].item_name, name ) == 0 ) {
and
if ( strcmp( answer, "yes" ) == 0 ) {
Also calls of the function fflush with the stream stdin
fflush (stdin);
has undefined behavior. Remove such calls.
Pay attention to that the function declaration does not make a great sense relative to its definition because within the function there is used only the first element of the passed array.
CodePudding user response：
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".
CodePudding user response：
Where to start...I feel like its the 80's again
buffer overruns = BAD
scanf does no validation
there are other issues with your code I leave that as an exercise for you to figure out the differences.
here is a more functional version ...
struct category {
const char* item_name; // const is good
int item_quantity;
double item_price; // float was too small
};
void items_search(category Category[])
{
char answer[4] = { "\0" }; // buffer was too small before
int number = 0;
int item_quantity = 0;
char name[200] = { "\0" };
int item = -1, Index = 0;;
printf("What are you looking for?\n");
scanf_s("9s", &name, _countof(name)); // validation!
bool loop = true;
for (; nullptr != Category[Index].item_name && loop; Index ) {// does lower case comparison
if (_stricmp(Category[Index].item_name, name) == 0) {
item = Index;
loop = false;
}
}
if (-1 != item) { // did we find the item ?
printf("What is the desired number of the item?");
scanf_s("%d", &number);
fflush(stdin);
if (number <= Category[item].item_quantity) { // you only accepted exact answer
printf("Order is possible. Should the order be placed?\n");
scanf_s("%3s", &answer, _countof(answer));
fflush(stdin);
if (_stricmp(answer, "yes") == 0) {
puts("The order is placed");
}
else {
puts("Order was cancelled");
}
}
else {
printf("There are only %d pieces of the item. Should the order be canceled ? \n", Category[item].item_quantity);
scanf_s("%3s", &answer, _countof(answer));
fflush(stdin);
if (_stricmp(answer, "yes") == 0) {
puts("Order was cancelled. Should be bought later?");
scanf_s("%3s", &answer, _countof(answer));
fflush(stdin);
if (_stricmp(answer, "yes") == 0) {
puts("The order is placed");
}
else {
puts("The order was cancelled");
}
}
else {
puts("The order is placed");
}
}
} else {
printf("The specified item does not exist\n");
}
}
the rest should be clear
Also in main
struct category laptops[] = {
{"Asus_365", 7, 499.00},
{"Lenovo_L49", 30, 699.91},
{"HP_Alien", 20, 649.99},
{"Acer Alpha touch", 10, 899.99},
{nullptr,0,0 }
};
items_search(laptops); // see items_search above