Zixuan Zhang Zixuan Zhang - 1 month ago 15
C Question

c getting Segmentation fault

I am new to C, and I have been writing a simply script like below:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

typedef enum {FALSE, TRUE} bool;

// item_t: information for a single item to be purchased
typedef struct _item_t {
char name[50];
int price;
int quantity;
} item_t;

// cart_t: information for the shopping cart
typedef struct _cart_t {
item_t *items[10];
int size;
} cart_t;

// Returns index of item with name `item_name` in the `items` array inside of `cart`. Returns -1 if not found.
int find_item(cart_t *cart, const char* item_name) {
int i;
for(i=0;i<sizeof(*cart);++i){
if(strcmp(cart->items[i]->name,item_name) == 0){
return i;
}
}

if(i == sizeof(*cart) -1)
return -1;
return 0;
}

// Adds item to cart's items array, if the cart is not full. Returns `FALSE` if the cart is full, and returns `TRUE` otherwise.
bool add_new_item(cart_t *cart, item_t *item) {
if(10 == cart->size)
return FALSE;
else{
int i = cart->size;
cart->items[i] = item;
cart->size +=1;
return TRUE;
}
}

// Increases quantity by one of item with name `item_name` inside of `cart`, if such an item exists.
void add_existing_item(cart_t *cart, const char* item_name) {
int i = find_item(cart,item_name);
if(i == -1)
return;
else{
cart->items[i]->quantity += 1;
}
}

// Removes item with name from cart's items array. Returns a pointer to the removed item, and shifts the items to the right of the deleted item in the `items` array
// left to fill in the gap.
// If the item is not found, return NULL.
item_t* remove_item(cart_t *cart, char *remove_me) {
item_t *p;
int j = 0;
int i = find_item(cart,remove_me);
if(i == -1)
return NULL;
else{

if(i == cart->size-1){
p = cart->items[j];
cart->items[j] = NULL;
}else{
p = cart->items[j];
for(j=i;j<cart->size-1;++j){
cart->items[j] = cart->items[j+1];
}
cart->items[cart->size-1] = NULL;
cart->size -= 1;
}
return p;
}
}

// If item with name `name` is found inside of `cart`, change its price and quantity to `price` and `quantity`.
void update_item_in_cart(cart_t *cart, const char* name, int price, int quantity) {
int i = find_item(cart,name);
if(i == -1)
return;
else{
cart->items[i]->quantity = quantity;
cart->items[i]->price = price;
}
}

// Returns total quantity of all items in cart.
int num_items(cart_t *cart) {
int total;
int i;
for(i=0;i<sizeof(*cart);++i){
total += cart->items[i]->quantity;
}
return total;
}

// Determines and returns the total cost of items in cart.
int total_cost(cart_t *cart) {
int total;
int i;
for(i=0;i<sizeof(*cart);++i){
total += cart->items[i]->quantity * cart->items[i]->price;
}
return total;
}

//print item's info and cost.
void print_item_cost(item_t *item) {
printf("%s %d @ $%d = $%d\n", item->name, item->quantity,
item->price, (item->price * item->quantity));
}

//print the shopping cart info
void print_total(cart_t *cart) {
int i = 0;
int cost = 0;

printf("Number of Items: %d\n\n", num_items(cart));

if (cart->size > 0) {
for (i = 0; i < cart->size; ++i) {
print_item_cost(cart->items[i]);
}
}

else {
printf("SHOPPING CART IS EMPTY\n");
}

cost = total_cost(cart);

printf("\nTotal: $%d\n", cost);
}

//this function clears items in the cart
void clear_cart(cart_t *cart) {
int i = 0;
for (i = 0; i < cart->size; i++) {
free(cart->items[i]);
}
cart->size = 0;
}

//a menu of options to manipulate the shopping cart
void print_menu(cart_t *cart) {
char choice = ' ';
char name[50] = "";
int price = 0;
int quantity = 0;
int i = 0;
char c = ' ';
item_t *item;

while(choice != 'q') {

printf("MENU\n");
printf("a - Add item to cart\n");
printf("r - Remove item from cart\n");
printf("c - Change item quantity\n");
printf("o - Output shopping cart\n");
printf("q - Quit\n\n");

while (choice != 'a' &&
choice != 'r' &&
choice != 'c' &&
choice != 'o' &&
choice != 'q') {
printf("Choose an option: ");
scanf(" %c", &choice);
printf("\n");
}

switch (choice) {
case 'a':
while ((c = getchar()) != EOF && c != '\n');

printf("ADD ITEM TO CART\n");
printf("Enter the item name: ");
fgets(name, 50, stdin);
printf("\n");

i = find_item(cart, name);
if (i != -1) {
printf("Item found. Updating quantity of item. \n");
add_existing_item(cart, name);

choice = ' ';
printf("\n");
break;
}

if (cart->size >= 10) {
printf("Cart full. Cannot add new item. \n");

choice = ' ';
printf("\n");
break;
}

printf("Enter the item price: ");
scanf("%d", &price);
printf("\n");

printf("Enter the item quantity: ");
scanf("%d", &quantity);
printf("\n");


item = (item_t*)malloc(sizeof(item_t));
strcpy(item->name, name);
item->price = price;
item->quantity = quantity;
add_new_item(cart, item);

choice = ' ';
printf("\n");

break;

case 'r':
while ((c = getchar()) != EOF && c != '\n');

printf("REMOVE ITEM FROM CART\n");
printf("Enter name of item to remove: ");
fgets(name, 50, stdin);
printf("\n");

item = remove_item(cart, name);
if (item == NULL) {
printf("Item not found. \n");
}
else {
printf("Item removed. \n");
free(item);
}

choice = '\0';
printf("\n");
break;

case 'c':

while ((c = getchar()) != EOF && c != '\n');

printf("CHANGE ITEM QUANTITY\n");
printf("Enter the item name: ");
fgets(name, 50, stdin);
printf("\n");

i = find_item(cart, name);

if (i == -1) {
printf("Item not found. \n");
choice = ' ';
printf("\n");
break;
}

printf("Enter the item price: ");
scanf("%d", &price);
printf("\n");


printf("Enter the new quantity: ");
scanf("%d", &quantity);
printf("\n");

update_item_in_cart(cart, name, price, quantity);

choice = '\0';
printf("\n");

break;

case 'o':
while ((c = getchar()) != EOF && c != '\n');
printf("OUTPUT SHOPPING CART\n");
print_total(cart);
choice = ' ';
printf("\n");
break;

}
}
clear_cart(cart);
}

int main(){
// malloc
cart_t *cart;
cart = (cart_t*)malloc(sizeof(cart_t));
cart->size = 0;

print_menu(cart);

//free
free(cart);
return 0;
}


It may look long, but the idea is simply, it simulate item in a shopping cart, it takes in command to add items/remove items/change quantity/display current cart information. However, at the very beginning of debugging, when I do following in command window:

$ ./try
MENU
a - Add item to cart
r - Remove item from cart
c - Change item quantity
o - Output shopping cart
q - Quit

Choose an option: a

ADD ITEM TO CART
Enter the item name: tira

Segmentation fault (core dumped)


I am getting segmentation fault. I am wondering what could be the bug here? I am really new to C, and not sure about the correctness of pointer stuff in my code.

PS:
Thanks for the reply, and yes it helps me solve the bugs in add_new_item function. However, when I debug remove_item, it terminate again with segmentation fault, the command line looks like below:

$ ./try
MENU
a - Add item to cart
r - Remove item from cart
c - Change item quantity
o - Output shopping cart
q - Quit

Choose an option: a

ADD ITEM TO CART
Enter the item name: shoes

Enter the item price: 1

Enter the item quantity: 2


MENU
a - Add item to cart
r - Remove item from cart
c - Change item quantity
o - Output shopping cart
q - Quit

Choose an option: r

REMOVE ITEM FROM CART
Enter name of item to remove: shoes

Item removed.

MENU
a - Add item to cart
r - Remove item from cart
c - Change item quantity
o - Output shopping cart
q - Quit

Choose an option: o

OUTPUT SHOPPING CART
Segmentation fault (core dumped)

Answer

One problem at least is in the find_item function. sizeof(*cart) has nothing to do with the actual number of items in the cart,, sizeof returns the number of bytes in the data structure. In this case, that's at least 44 bytes (assuming 4 byte int and 4 byte pointers). You should change this function to something like this:

// Returns index of item with name `item_name` in the `items` array inside of `cart`. Returns -1 if not found.
int find_item(cart_t *cart, const char* item_name) {
    int i;
    for(i=0;i<cart->size;++i){
        if(strcmp(cart->items[i]->name,item_name) == 0){
            return i;
        }
    }

    return -1;  // if we get here, then no items were found, so return -1.
                // I don't understand your logic in returning 0, since 0 is a valid item index
}

As noted by @Fang, confusing the number of items in your cart (cart->size) with sizeof(*cart) is a common mistake throughout your code. This will need to be changed any place you are trying to iterate through your items.