why am i getting a segmentation fault core dumped for my inventory management codehttps://onlinegdb.com/ryg1UogVw

+3 votes
asked Sep 5 by atulsangahvi (200 points)
Hi

please help why segmentation fault for my code ?

https://onlinegdb.com/ryg1UogVw

3 Answers

0 votes
answered Sep 5 by xDELLx (2,950 points)

When asking for questions , it would be helpful to mention when the error occurs , what steps were performed prior to error.

It will give a headstart for analysing the code & is also polite.

That aside , change your print () function, use below



void print(void)
{
    struct part *p;
    printf("Part Number   Part Name                  Quantity on Hand\n");
    for (p = inventory; p !=NULL; p = p->next)
        printf("%7d      %-25s%11d\n", p->number,p->name, p->on_hand);
        
}
/* output below

Enter operation code : p                                                                                                       

Part Number   Part Name                  Quantity on Hand                                                                      

     12      ab                                10                                                                              

   1111      aaa                               11                                                                              

*/

+1 vote
answered Sep 9 by Peter Minarik (7,830 points)
edited Sep 9 by Peter Minarik

Notes

I agree with what xDELLx said about how to change your code to print the right thing.

Also, as he said, it's advised to tell the steps you took to end up with the error.

After applying the fix suggested by xDELLx I wasn't able to run into a Segmentation Fault.

Improvement Opportunities

Use typedef

You can use typedef to make your code easier to read and shorter too. For instance with typedef struct part would look something like this:

typedef struct
{
    int number;
    char name[NAME_LEN+1];
    int on_hand;
    struct part *next;
} Part;

Part *inventory = NULL; /* points to first part */
Part *find_part(int number);

So instead of writing "struct part" you can just reference your type via "Part".

A Slight Variant of Code

I'm sharing my take on the insert/find problem.

The main difference is that the search function (tryFind()) has an out parameter that returns the pointer to the found part. If not found, it returns the pointer where it should be inserted.

To work with this, insertAt() takes an argument that tells where the new part should be inserted. 

// Tries to find a part based on it's number.
// If found, returns true (non-zero) and *part points to the found part.
// If not found, returns false (zero) and *part points to the part where a new one with the given number could be inserted
int tryFind(int number, Part ** part)
{
    Part * cur = inventory;
    Part * prev = NULL;
    while (cur != NULL && number > cur->number)
    {
        cur = cur->next;
        prev = prev != NULL ? prev->next : inventory;
    }
    
    // Part number found. Part points
    if (cur && cur->number == number)
    {
        *part = cur;
        return 1;
    }

    // Part number not found
    *part = prev;
    return 0;
}

// Insert a new part with (number, name, onHand) at a given position
// position: the part, after the new part should be inserted. Provide NULL if the part needs to go to the head of the list.
// number:   the part number
// name:     the name of the part
// onHand:   the number of parts availabel on hand
void insertAt(Part * position, int number, const char * name, int onHand)
{
    Part * newPart = (Part*)malloc(sizeof(Part));
    newPart->number = number;
    strncpy(newPart->name, name, sizeof(newPart->name));
    newPart->on_hand = onHand;

    if (position == NULL)
    {
        newPart->next = inventory;
        inventory = newPart;
    }
    else
    {
        newPart->next = position->next;
        position->next = newPart;
    }
}

So with these two functions above a simple insert would look something like this:

void insertHelper(int number, const char * name, int count)
{
    Part * part;
    if (tryFind(number, &part))
    {
        printf("Part already found: %p\n", part);
        return;
    }
    
    insertAt(part, number, name, count);
}
–1 vote
answered Sep 19 by Arti Ahirwar (200 points)

I agree with what xDELLx said about how to change your code to print the right thing.

Also, as he said, it's advised to tell the steps you took to end up with the error.

After applying the fix suggested by xDELLx I wasn't able to run into a Segmentation Fault.

Improvement Opportunities

Use typedef

You can use typedef to make your code easier to read and shorter too. For instance with typedef struct part would look something like this:

typedef struct
{
    int number;
    char name[NAME_LEN+1];
    int on_hand;
    struct part *next;
} Part;

Part *inventory = NULL; /* points to first part */
Part *find_part(int number);

So instead of writing "struct part" you can just reference your type via "Part".

A Slight Variant of Code

I'm sharing my take on the insert/find problem.

The main difference is that the search function (tryFind()) has an out parameter that returns the pointer to the found part. If not found, it returns the pointer where it should be inserted.

To work with this, insertAt() takes an argument that tells where the new part should be inserted. 

// Tries to find a part based on it's number.
// If found, returns true (non-zero) and *part points to the found part.
// If not found, returns false (zero) and *part points to the part where a new one with the given number could be inserted
int tryFind(int number, Part ** part)
{
    Part * cur = inventory;
    Part * prev = NULL;
    while (cur != NULL && number > cur->number)
    {
        cur = cur->next;
        prev = prev != NULL ? prev->next : inventory;
    }
    
    // Part number found. Part points
    if (cur && cur->number == number)
    {
        *part = cur;
        return 1;
    }

    // Part number not found
    *part = prev;
    return 0;
}

// Insert a new part with (number, name, onHand) at a given position
// position: the part, after the new part should be inserted. Provide NULL if the part needs to go to the head of the list.
// number:   the part number
// name:     the name of the part
// onHand:   the number of parts availabel on hand
void insertAt(Part * position, int number, const char * name, int onHand)
{
    Part * newPart = (Part*)malloc(sizeof(Part));
    newPart->number = number;
    strncpy(newPart->name, name, sizeof(newPart->name));
    newPart->on_hand = onHand;

    if (position == NULL)
    {
        newPart->next = inventory;
        inventory = newPart;
    }
    else
    {
        newPart->next = position->next;
        position->next = newPart;
    }
}

So with these two functions above a simple insert would look something like this:

void insertHelper(int number, const char * name, int count)
{
    Part * part;
    if (tryFind(number, &part))
    {
        printf("Part already found: %p\n", part);
        return;
    }
    
    insertAt(part, number, name, count);
commented Sep 19 by xDELLx (2,950 points)
isnt it copy paste of Peter Minarik answer ?
Welcome to OnlineGDB Q&A, where you can ask questions related to programming and OnlineGDB IDE and and receive answers from other members of the community.
...