C segmentation fault when trying to implement LinkedList

Question

I've got the following code:

#include <stdio.h>
#include <stdlib.h>
#define MAXN 100
typedef int key;
typedef int data;
struct list * createElement(key k, data info);

struct list{
    key k;
    data info;
    struct list *next;
};
struct list *L;
void init(key k, data info)
{
    L = createElement(k, info);
}
struct list * createElement(key k, data info)
{
    struct list *temp;
    temp = (struct list *) malloc(sizeof(*temp));
    temp->k = k;
    temp->info = info;
    return temp;
}
void insert(struct list * element)
{
    element->next = L;
    L = element;
}
void insertBefore(struct list * element, key k)
{
    struct list * currentElement = L;
    while(currentElement != NULL)
    {
        if(currentElement->k == k)
        {
            struct list *temp = currentElement;
            currentElement = element;
            currentElement->next = temp;
            return;
        }
        currentElement = currentElement->next;
    }

}
void insertAfter(struct list * element, key k)
{
    struct list * currentElement = L;
    while(currentElement != NULL)
    {
        if(currentElement->k == k)
        {
            struct list *temp = currentElement->next;
            currentElement->next = element;
            element->next = temp;
            return;
        }
        currentElement = currentElement->next;
    }
}
void deleteElement(struct list * element)
{
    struct list * currentElement = L;
    while(currentElement != NULL)
    {
        if(currentElement == element)
        {
            struct list * temp = currentElement;
            currentElement = currentElement->next;
            free(temp);
            return;
        }
        currentElement = currentElement->next;
    }
}
struct list * getElementByKey(key k) {
    printf("\n1");
    struct list *currentElement = L;
    printf("2");
    while(currentElement != NULL)
    {
        printf("3");
        if(currentElement->k == k)
        {
            printf("4");
            return currentElement;
        }
        printf("5");
        currentElement = currentElement->next;
        printf("6");
    }
    printf("There is no such element in the list");
}
struct list * pop()
{
    struct list *element = L;
    L = L->next;
    return element;
}
int main()
{
    init(0, 13);
    struct list * element = createElement(5, 155);
    insert(element);
    struct list * k = createElement(7, 243);
    insert(k);
    //insertBefore(createElement(3, 100), 5);
    printf("The first element value is: %d", pop()->info);
    printf("The second element value is: %d", pop()->info);
    printf("The  element value is: %d", getElementByKey(5)->info);

    return 0;
}

So when i execute it the debugger gives me segmantation fault on line 84 which is if(currentElement->k == k) on method getElementByKey. I know that i'm trying to access an unexisting element(because i use the pop method and it removes the first element in the list) but it should print me a warning message. it seems that there is a problem in the previous element's key or something that i failed to notice.

Nice typo in the title, btw ... ;-)

alk
– alk

2013-04-06 12:58:16 +00:00
Commented Apr 6, 2013 at 12:58 — alk
– alk, Commented Apr 6, 2013 at 12:58
yes, thanks just noticed it

user1113314
– user1113314

2013-04-06 13:07:53 +00:00
Commented Apr 6, 2013 at 13:07 — user1113314
– user1113314, Commented Apr 6, 2013 at 13:07

Julien Fouilhé · Accepted Answer · 2013-04-06 13:20:04Z

3

In your createElement function, you forgot to initialize the next pointer to NULL.

struct list * createElement(key k, data info)
{
    struct list *temp;
    temp = (struct list *) malloc(sizeof(*temp));
    temp->k = k;
    temp->info = info;
    temp->next = NULL;
    return temp;
}

So when you're browsing your list, it never finds the last element of the list because next pointer never equals NULL, and goes at some random position in the memory, trying to access to parts it is not allowed to do so.

edited Apr 6, 2013 at 13:20

answered Apr 6, 2013 at 13:13

Julien Fouilhé

2,6783 gold badges33 silver badges58 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

user1113314 Over a year ago

Thanks! That solved my problem. I thought the compiler automatically assigns it to NULL and threfore need not write it(I got a PHP backgroud and that's how things work there). That sure gave me a lesson.

Julien Fouilhé Over a year ago

Yes, be sure to initialize everything in C, you can lose a bunch of time debugging a non-initialized variable.

alk · Accepted Answer · 2013-04-06 12:57:26Z

1

When deleting an element (using deleteElement()) you miss to re-initialise L.

The segmenataion violation in if(currentElement->k == k)most likey dues to currentElement referring to an invalid memory address. As you test if against NULL some lines before line 84, it can only be the case that currentElement had either been assigned an invalid address explicitly or the address it points had been freed already.

From the code shown in deleteElement() I assume the latter.

edited Apr 6, 2013 at 12:57

answered Apr 6, 2013 at 12:51

alk

71.2k10 gold badges112 silver badges274 bronze badges

2 Comments

user1113314 Over a year ago

What do you mean by "When deleting an element (using deleteElement()) you miss to re-initialise L."? I dont have much experience with C and pointers. I am not using the deleteElement method so it's not liked to it this particular error. I will try to see where currentElement links to bad memory

Fred Over a year ago

deleteElement() is not correct, and you'll need to fix it before you use it. Suppose you delete the element to which L points. The entire list is gone because L points to memory that has been free()ed.

BLUEPIXY · Accepted Answer · 2013-04-06 16:44:18Z

1

two problem

1)at createElement()

need

temp->next = NULL;

2)at insertBefore()

not change pointer of list

currentElement = element;

e.g)

void insertBefore(struct list * element, key k)
{
    struct list * currentElement = L; //add case of L->k
    while(currentElement != NULL)
    {
        if(currentElement->next && currentElement->next->k == k)
        {
            element->next = currentElement->next;
            currentElement->next = element;
            return;
        }
        currentElement = currentElement->next;
    }
}

edited Apr 6, 2013 at 16:44

answered Apr 6, 2013 at 13:18

BLUEPIXY

40.2k7 gold badges37 silver badges73 bronze badges

3 Comments

BLUEPIXY Over a year ago

getElementByKey(): return value is not determined when not find,.

user1113314 Over a year ago

This is a good way to implement it, but mine is identical and should work properly.

BLUEPIXY Over a year ago

@user1113314, do you think when currentElement = element; Who is pointing to the element ?

Collectives™ on Stack Overflow

C segmentation fault when trying to implement LinkedList

3 Answers 3

2 Comments

2 Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

2 Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related