2
votes

I have written the following code segment and am having trouble understanding why it would not get to the last printf line. I get a segfault immediately after line 4. The kill_char is just used to kill the 'enter' character added in the previous scanf. Any help would be greatly appreciated, thanks!

int remove = 0;
char kill_char = 'a';
printf("Enter the product number to be removed: ");
scanf("%d", &remove);
scanf("%c", &kill_char);
printf("Does not get here");

EDIT: Full code is as follows, with the error in the removeProduct function

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

struct product_node {
char *supply_type;
long number;
char *description;
float price;
int quantity_bought;
float retail_price;
int quantity_sold;
struct product_node *next;
};

struct product_node *head;//declaring head out here 
//allows the list to be in scope for the functions

/*Function Prototypes*/
void addProduct();
void removeProduct();
void listProduct();
void listSupplierTypes();
void supplierTypeProfit();
void totalProfit();

void addProduct(){
    char kill_char = 'a';//used to kill the enter characters
    struct product_node *new_node;
    new_node = malloc(sizeof(struct product_node));

    printf("\nEnter a string for type: ");
    scanf( "%s", &(*new_node).supply_type);
    scanf("%c", &kill_char);
    printf("Enter the product number: ");
    scanf("%ld", &(*new_node).number);
    scanf("%c", &kill_char);
    printf("Enter the description: ");
    scanf("%s", &(*new_node).description);
    scanf("%c", &kill_char);
    printf("Enter the wholesale price: ");
    scanf("%f", &(*new_node).price);
    scanf("%c", &kill_char);
    printf("Enter the quantity bought: ");
    scanf("%d", &(*new_node).quantity_bought);
    scanf("%c", &kill_char);
    printf("Enter the retail price: ");
    scanf("%f", &(*new_node).retail_price);
    scanf("%c", &kill_char);
    printf("Enter the quantity sold: ");
    scanf("%d", &(*new_node).quantity_sold);
    scanf("%c", &kill_char);

    struct product_node *walker;
    walker = head;
    int can_insert = 1;
    while (!(walker == NULL))
    {
        if (((*walker).number == (*new_node).number) && ((*walker).supply_type == (*new_node).supply_type))
        {
            can_insert = 0;
        }
        walker = (*walker).next;
    }

    if (can_insert==1)
    {
        (*new_node).next = head;
        head = new_node;
        printf("Insertion Successful");
    }
    else
    {
        printf("\nERROR INSERTING:This product name and number is already in the list\n");
    }
    free(new_node);
}
void removeProduct(){
    int remove = 0;
    char kill_char = 'a';
    printf("Enter the product number to be removed: ");
    scanf("%d", &remove);
    scanf("%c", &kill_char);
    printf("Does not get here");
    struct product_node *walker;
    struct product_node *prev;
    prev = head;
    walker = (*head).next;

    if ((*prev).number == remove)
    {
    head = walker;
    }//points head to second node to remove first

    while (!(walker = NULL))
    {
        if ((*walker).number == remove)
        {
            (*prev).next = (*walker).next;
        }
    }
}
void listProduct(){
    printf("Still unsure what defines a supplier...");
}
void listSupplierTypes(){
    printf("Same as above");
}
void supplierTypeProfit(){
    printf("Again");
}
void totalProfit(){
    float total = 0.0;
    struct product_node *walker;
    walker = head;
    while(!(walker == NULL))
    {
        total += ((float)(*walker).quantity_sold * (*walker).retail_price) - ((float)(*walker).quantity_bought * (*walker).price);
        walker = (*walker).next;
    }
    printf("Total Profit is: $%.2f\n", total);
}

int main()
{
    head = NULL;

    char *temp_type;
    char *temp_description;
    int temp_number, temp_quantity_bought, temp_quantity_sold;
    float temp_price, temp_retail_price;

    while(!feof(stdin))
    {
        scanf( "%s %ld %s %f %d %f %d\n", &temp_type, &temp_number, &temp_description, &temp_price, &temp_quantity_bought, &temp_retail_price, &temp_quantity_sold);

        struct product_node *new_node;
        new_node = malloc(sizeof(struct product_node));
        (*new_node).next = head;
        head = new_node;

        (*head).supply_type = temp_type;
        (*head).number = temp_number;
        (*head).description = temp_description;
        (*head).price = temp_price;
        (*head).quantity_bought = temp_quantity_bought;
        (*head).retail_price = temp_retail_price;
        (*head).quantity_sold = temp_quantity_sold;
    }

    freopen("/dev/tty", "rw", stdin);

    int done=0;
    int selection=0;

    while (!done)
    {
        printf("\nMENU OPTIONS:\n");
        printf("1. Add a product number\n");//Okay
        printf("2. Remove a product number\n");
        printf("3. List the products for a supplier\n");
        printf("4. List all unique supplier types\n");
        printf("5. Show profit margin for a specific supplier type\n");
        printf("6. Show total profit\n");//Okay
        printf("7. Quit\n");//Okay
        printf("Enter a selection (1-7): ");

        scanf("%d", &selection);
        char garbage = 'a';
        scanf("%c", &garbage);

        switch(selection){
        case 1:
            addProduct();
            break;
        case 2:
            removeProduct();
            break;
        case 3:
            listProduct();
            break;
        case 4:
            listSupplierTypes();
            break;
        case 5:
            supplierTypeProfit();
            break;
        case 6:
            totalProfit();
            break;
        case 7:
            done = 1;
            break;
        default:
            printf("Invalid selection.\n");
            break;
        }
    }
}
2
Your code seems fine. Problem is likely in the code that you haven't shown to us.P.P
Your code is fine - show us the actual problem, please!Carl Norum
Is "line 4" the first scanf call?Keith Thompson
Yes, the after first scanf call is where the segfault comes in.WhatsInAName

2 Answers

3
votes

remove is the name of a standard function, declared in <stdio.h>. Defining your own object or other entity with the same name has undefined behavior. The call may be trying to store an int value at the address of the remove() function.

Try picking a different name.

UPDATE: I think I was mistaken. Function names defined in standard headers are reserved for use as identifiers with external linkage; they're also reserved for use as a macro name and as an identifier with file scope if the relevant header is #included. Neither should apply in your case. It's still a good idea to avoid defining such identifiers yourself, though.

Also, this probably isn't related to the symptom you're seeing, but

scanf("%d", &obj);

has undefined behavior if the input is a syntactically valid integer whose value is outside the range of int.

Execution does reach your "Does not get here" line. You're not seeing it because the buffered output isn't printed before the program dies. Change this:

printf("Does not get here");

to this:

printf("Does not get here\n");
fflush(stdout);

When I run your program under gdb, I see the seg fault at:

if ((*walker).number == remove)

I also get several warnings during compilation:

c.c: In function ‘addProduct’:
c.c:32:5: warning: format ‘%s’ expects argument of type ‘char *’, but argument 2 has type ‘char **’ [-Wformat]
c.c:38:5: warning: format ‘%s’ expects argument of type ‘char *’, but argument 2 has type ‘char **’ [-Wformat]
c.c: In function ‘main’:
c.c:134:9: warning: format ‘%s’ expects argument of type ‘char *’, but argument 2 has type ‘char **’ [-Wformat]
c.c:134:9: warning: format ‘%ld’ expects argument of type ‘long int *’, but argument 3 has type ‘int *’ [-Wformat]
c.c:134:9: warning: format ‘%s’ expects argument of type ‘char *’, but argument 4 has type ‘char **’ [-Wformat]

which could easily cause memory corruption. Fix those and see what happens.

UPDATE 2:

I don't know what other programs your code may still have, but this:

while (!(walker = NULL))
{  
    if ((*walker).number == remove)
    {  
        (*prev).next = (*walker).next;
    }
}

is almost certainly wrong. You're using an assignment = operator where you probably want an equality comparison ==. And after fixing that, the code would be clearer as follows:

while (walker != NULL)
{
    if (walker->number == remove)
    {
        prev->next = walker->next;
    }
}

That's just what jumped out at me when I took a very quick look after gdb told me the segfault was on the line if ((*walker).number == remove).

Try using a debugger yourself, fix one problem at a time, and pay attention to any compiler warnings.

0
votes

Your printf does not appear because it didn't flush from the buffer, just use a "\n" at the end of the string and you will see it:

printf("Does not get here\n");

And so, the error, is not at scanf, but at this line:

walker = (*head).next;

As I could see, the program may reach there while head is not allocated, so you can check it at the beginning of the function:

void removeProduct(){
    int remove = 0;
    char kill_char = 'a';
    if (head == NULL) {
        printf("No product to remove!\n");
        return;
    }

I'm not sure if there is any other errors, but this is the one I noticed.

BTW, you can avoid using kill_char by inserting a space at the beginning and end of format string on scanf:

scanf(" %d ", &remove);

It will skip all white characters (as tabs, spaces and line breakers). And, if you really just want to skip one, and only one, character, you can use * to ignore the match:

scanf("%d%*c", &remove);