Thread: Segmentation Fault with Linked Lists

  1. #1
    Registered User
    Join Date
    Aug 2006
    Posts
    62

    Segmentation Fault with Linked Lists

    Hey guys, I've been working on this program for a class and I'm getting a really strange error. Basically we had to create a dynamically linked list and then delete, modify, and insert elements. The list needs to stay sorted by part number. By that I mean if structure 1 has part number 1234, structure 2 has part number 2345, and structure 3 has part number 2000, structure 1 must point to structure 3, which must point to structure 2.

    Anyway, the highest value for part number that I have "hard coded" into the program is 3456, and the lowest is 1234. It seems that when I try to insert a value greater than 3456 at runtime the program crashes and I get a segmentation fault.

    Here's the program. I'm using Code::Blocks 8.02 w/ the default compiler, under Ubuntu 9.10.

    It is attached, or the code is below...

    On a sidenote, I am aware that using system() is poor practice, this is just the rough version as of now though...

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    struct PartsInfo{
        int part_number;
        int quantity;
        float price;
        struct PartsInfo *next;
    };
    
    void createData(struct PartsInfo *);
    void printData(struct PartsInfo *);
    void insertPart(struct PartsInfo *);
    void modifyPart(struct PartsInfo *);
    void deletePart(struct PartsInfo *);
    
    int main(){
        int choice = 0;
        struct PartsInfo *first = (struct PartsInfo *) malloc(sizeof(struct PartsInfo));
    
        createData(first);
    
        do {
            printf("\nWhat would you like to do?\n");
            printf("1. Search for a part\n");
            printf("2. Create a new part\n");
            printf("3. Modify an existing part\n");
            printf("4. Delete an existing part\n");
            printf("0. Exit\n\n");
            printf("->"); scanf("%d",&choice);
            //system("clear");
    
            switch (choice){
                case 1: printData(first);   break;
                case 2: insertPart(first);  break;
                case 3: modifyPart(first);  break;
                case 4: deletePart(first);  break;
                default: ;
            }
        } while (choice != 0);
    
        return 0;
    }
    
    void createData(struct PartsInfo *data_struct){
        struct PartsInfo *temp_addr;
        int i;
    
        data_struct->part_number = 1234;
        data_struct->quantity = 5;
        data_struct->price = 5.25;
    
        for (i = 0 ; i <= 1; ++i){
            if ((data_struct->next = (struct PartsInfo *) malloc(sizeof(struct PartsInfo))) != (struct PartsInfo *) NULL){
                temp_addr = data_struct;
                data_struct = data_struct->next;
                data_struct->part_number = temp_addr->part_number + 1111;
                data_struct->quantity = temp_addr->quantity + 2;
                data_struct->price = temp_addr->price + 2.25;
            }
            else printf("Initial Memory Allocation Failed!");
        }
        data_struct->next = (struct PartsInfo *) NULL;
    }
    
    void printData(struct PartsInfo *data_struct){
        int part_number;
        printf("\n\nPlease enter the four digit part number(enter 0 for all parts): "); scanf("%d",&part_number);
        do {
            if (data_struct->part_number == part_number || part_number == 0){
                printf("Part Data\n");
                printf("Part Num  : %d\n",data_struct->part_number);
                printf("Part Qty  : %d\n",data_struct->quantity);
                printf("Part Cost : %.2f\n\n",data_struct->price);
                if (part_number != 0) break;
            }
            data_struct = data_struct->next;
        } while (data_struct != (struct PartsInfo *) NULL);
        if (data_struct == (struct PartsInfo *) NULL && part_number != 0)
            printf("Error! Record for part number %d not found!",part_number);
    }
    
    void insertPart(struct PartsInfo *data_struct){
        struct PartsInfo *next_addr, *new_addr;
        int part_number;
        printf("\n\nCreate New Part\n");
        printf("Please enter the part number: "); scanf("%d",&part_number);
    
        if ((new_addr = (struct PartsInfo *) malloc(sizeof(struct PartsInfo))) != (struct PartsInfo *) NULL){
            do {
                if (data_struct->next != (struct PartsInfo *) NULL)
                    next_addr = data_struct->next;
                else next_addr = (struct PartsInfo *) NULL;
    
                if (part_number > data_struct->part_number &&
                    ((next_addr->part_number > part_number) || next_addr == (struct PartsInfo *) NULL)){
                    data_struct->next = new_addr;
                    data_struct = new_addr;
                    data_struct->part_number = part_number;
                    printf("Please enter the qty  : "); scanf("%d",&data_struct->quantity);
                    printf("Please enter the cost : "); scanf("%f",&data_struct->price);
                    data_struct->next = next_addr;
                    break;
                }
    
                data_struct = data_struct->next;
            } while (data_struct != (struct PartsInfo *) NULL);
        }
        else printf("Error! Allocation Failed!");
    }
    
    void modifyPart(struct PartsInfo *data_struct){
        int part_number;
        printf("\n\nModify an Existing Part\n\n");
        printf("Please enter the four digit part number: "); scanf("%d",&part_number);
    
        do {
            if (data_struct->part_number == part_number){
                printf("Please enter part qty  : "); scanf("%d",&data_struct->quantity);
                printf("Please enter part cost : "); scanf("%f",&data_struct->price);
                break;
            }
            data_struct = data_struct->next;
        } while (data_struct != NULL);
    
        if (data_struct == NULL)
            printf("Error! Record for part number %d not found!",part_number);
    }
    
    void deletePart(struct PartsInfo *data_struct){
        int part_number;
        struct PartsInfo *temp_addr = NULL;
        printf("\n\nDelete a Part\n\n");
        printf("Please enter the four digit part number: "); scanf("%d",&part_number);
    
        do {
            temp_addr = data_struct;
            data_struct = data_struct->next;
            if (data_struct->part_number == part_number){
                printf("Deleting structure from list...");
                temp_addr->next = data_struct->next;
                free(data_struct);
                printf("Done!");
                break;
            }
        } while (data_struct != NULL);
        if (data_struct == NULL)
            printf("Error! Record for part number %d not found!",part_number);
    }

  2. #2
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    I wouldn't squish my number reading into the same function as my insert function. Ideally, it should be something like this:
    Code:
    void InsertPart( ...list head..., partnumber )
    As for this:
    Code:
    void insertPart(struct PartsInfo *data_struct){
    You are more than likely going to have a fault when you try to insert a part that comes before everything else in your list. Or at least it's not going to update your list correctly. You need a pointer to the pointer to update, if you want to ever be able to change the contents of that first pointer. Meaning, if your first part is 2, and you want to add 1, you won't be able to do that otherwise.
    Code:
    void insertParts( struct PartsInfo **list );
    ...
    insertParts( &mylistofparts );
    Back to the point I mentioned earlier:
    Code:
        printf("Please enter the part number: "); scanf("%d",&part_number);
    
        if ((new_addr = (struct PartsInfo *) malloc(sizeof(struct PartsInfo))) != (struct PartsInfo *) NULL){
    Why are you scanning for a number here, and then allocating automatically? Shouldn't you first see if the part is already in the list, or did you already do that some place else? There's no point in allocating until you know it's not already there.
    Code:
                if (data_struct->next != (struct PartsInfo *) NULL)
    You don't need to typecast NULL.

    It would be cleaner to do something like this:
    Code:
    void foo( struct node **list, int pn )
    {
        if( list )
        {
            struct node *n = *list;
            if( n->partnumber < pn ) /* prepend */
            {
                ...allocate a new node...
                ...put a part number in it...
                newnode->next = n;
                *list = newnode;
            }
            else
            {
                while( n->next && n->next->partnumber > pn )
                    n = n->next;
    
                ...allocate a new node...
                ...put a part number in it...
                newnode->next = n->next;
                n->next = newnode;
            }
        }
    }
    You can clean that up a bit more, by only allocating in one spot, by doing a couple of extra checks (which you'd want to actually do here anyway, such as, if n's part number is the same as what you want to insert, then don't allocate, and stop looking. But that's the general idea.


    Quzah.
    Hope is the first step on the road to disappointment.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 5
    Last Post: 11-04-2006, 06:39 PM
  2. Segmentation faults on Linked Lists. (Please help!!)
    By summerrainx in forum C++ Programming
    Replies: 3
    Last Post: 03-19-2005, 07:23 AM
  3. Locating A Segmentation Fault
    By Stack Overflow in forum C Programming
    Replies: 12
    Last Post: 12-14-2004, 01:33 PM
  4. Linked Lists 101
    By The Brain in forum C++ Programming
    Replies: 5
    Last Post: 07-24-2004, 04:32 PM
  5. need help w/ linked lists
    By MKashlev in forum C++ Programming
    Replies: 11
    Last Post: 08-05-2002, 08:57 PM