Push on position and pop from position functions in double conected list

Question

I have a problem with my double conected list I have to add 2 functions. First which adds element on specified position (if position does not exsist it has to add an element at the end of list), example:

Elements: 1 2 3
Add on position (we start numbering from 0): 1 element: 21
Elements after adding: 1 21 2 3

Second which deletes element on specified position (if position does not exsist it has to do nothing), example:

Elements: 1 2 3
Delete position: 1
Elements after deleting: 1 3

Here's what I've tried to write, the problem is that my program is crashing when I try to add or delete element to/from list. Thank you for your time and help!

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

struct ellist2c {
    int x;
    struct ellist2c * right;
    struct ellist2c * left;

    };
struct list2c {
    struct ellist2c * begining;
    struct ellist2c * end;
    int position;
};
void push(int newdata, struct list2c * list) {
    struct ellist2c * newEl = (struct ellist2c * ) malloc(sizeof(struct ellist2c ));
    newEl -> x = newdata;
    newEl -> left= NULL;
    if (list -> position == 0) {
        list -> begining= newEl;
        list -> end= newEl;
        newEl -> right= NULL;
    } else {
        newEl -> right= list -> end;
        list -> end-> left= newEl;
        list -> end= newEl;
    }
    list -> position++;
}
void pushonposition(int newData, int poz, struct list2c * list){
    struct ellist2c * sk = (struct ellist2c * ) malloc(sizeof(struct ellist2c ));
    sk -> x = newData;
    struct ellist2c * el = (struct ellist2c * ) malloc(sizeof(struct ellist2c ));
    el = list -> begining;
    if (poz == 0) {
        el -> x = newData;
        if (list -> begining == NULL) {
            el -> right = NULL;
            el -> left = NULL;
            list -> begining = el;
            list -> end = el;
        } else {
            el -> left = NULL;
            list -> begining -> left = el;
            el -> right = list -> begining;
            list -> begining = el;
        }
        list -> position++;
        return;
    }
    if (poz >= list -> position - 1) {
        el -> x = newData;
        if (list -> begining == NULL) {
            el -> right = NULL;
            el -> left = NULL;
            list -> begining = el;
            list -> end = el;
        } else {
            el -> right = NULL;
            el -> left = list -> end;
            list -> end -> right = el;
            list -> end = el;
        }
        list -> position++;
        return;
    }
    for (int i = 0; i < poz; i++) {
        el = el -> right;
    }
    sk -> left = el -> left;
    struct ellist2c * before = (struct ellist2c * ) malloc(sizeof(struct ellist2c ));
    before = list -> begining;
    for (int i = 0; i < poz - 1; i++) {
        before = before -> right;
    }

    sk -> right = before -> right;
    before -> right = sk;
    sk -> right -> left = sk;
}
void popposition(struct list2c * list, int index) {
    struct ellist2c * el = (struct ellist2c * ) malloc(sizeof(struct ellist2c ));
    if (list -> begining == NULL) {
        return;
    }
    if (index == 0) {
        if (list -> begining == NULL) {
            return;
        }

        if (list -> begining == list -> end) {
            el = list -> end;
            free(el);
            list -> end = NULL;
            list -> begining = NULL;
            printf("Empty list");
        } else {
            el = list -> end;
            list -> end = el -> left;
            free(el);
            list -> end -> right = NULL;
        }
        list -> position--;
        return;
    }
    el = list -> begining;
    for (int i = 0; i < index; i++) {
        el = el -> right;
        if (el == NULL) {
            return;
        }
    }
    if (el != list -> end && el != list -> begining) {
        el -> right -> left = el -> left;
        el -> left -> right = el -> right;
        free(el);
        return;
    }
    if (el == list -> end) {
        list -> end = el -> left;
        list -> end -> right = NULL;
        free(el);
        return;
    }
    if (el == list -> begining) {
        list -> begining = el -> right;
        list -> begining -> left = NULL;
        free(el);
        return;
    }}
void print(struct list2c * list) {
        struct ellist2c * el = (struct ellist2c * ) malloc(sizeof(struct ellist2c ));
        el = list -> begining;
        while (el) {
            printf("%d\n", el -> x);
            el = el -> left;
}
}
int main(){
    struct list2c * list = (struct list2c * ) malloc(sizeof(struct list2c ));
    list -> begining= NULL;
    list -> end = NULL;
    list -> position = 0;
    push(5, list);
    push(6, list);
    push(7, list);
    pushonposition(1, 1, list);
    pushonposition(1, 1, list);
    popposition(1, list);
    print(list);
return (0);
}

yeah... Segmentation Fault? Try to to determine where exactly. — purec, May 27 '18 at 14:02
When you stepped through this program using your preferred debugger, where did you find the problem occurring? — Fantastic Mr Fox, May 27 '18 at 14:03
So.. it crashes upon the 'push(5, list);' call, and so all the subsequent calls are irrelevant, yes? — Martin James, May 27 '18 at 14:05
Note that the dot `.` and arrow `->` operators bind very tightly and should not be written with spaces around them. — Jonathan Leffler, May 27 '18 at 14:22
You should check that your memory allocations succeed before using the result. However, it is relatively unlikely that you're suffering from memory allocation failures, so it probably isn't directly the cause of your problems — but robust C code does ensure that memory is allocated successfully before using it. — Jonathan Leffler, May 27 '18 at 14:23
I've tried to debug and debugger says that there's problem with line `el->x = newData;` in PushOnPosition function, but i don't realy know why — Sz3jdii, May 27 '18 at 16:09

Dennis Vash · Accepted Answer · 2018-05-27T14:38:58.383

Your code is pretty messy and you have many unused variables and memory leaks.

I fixed the code a bit, you should start by rewriting the pop_position function:

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


#define EMPTY 0

typedef struct list_node {
    int x;
    struct list_node *first;
    struct list_node *last;
} *ListNode;

typedef struct list_master {
    ListNode first;
    ListNode last;
    int size;
} *ListMaster;

void push(int value, struct list_master *list) {
    ListNode node = malloc(sizeof(ListNode));
    node->x = value;
    node->last = NULL;
    if (list->size == EMPTY) {
        list->first = node;
        list->last = node;
        node->first = NULL;
    } else {
        node->first = list->last;
        list->last->last = node;
        list->last = node;
    }
    ++list->size;
}

void print(ListMaster list) {
    ListNode el;
    el = list->first;
    while (el) {
        printf("%d\n", el->x);
        el = el->last;
    }
}

void push_on_position(int newData, int position, ListMaster list) {
    ListNode sk = malloc(sizeof(ListNode));
    sk->x = newData;
    ListNode el = list->first;
    if (position == EMPTY) {
        el->x = newData;
        if (list->first == NULL) {
            el->last = NULL;
            el->first = NULL;
            list->first = el;
            list->last = el;
        } else {
            el->first = NULL;
            list->first->first = el;
            el->last = list->first;
            list->first = el;
        }
        ++list->size;
        return;
    }
    if (position >= list->size - 1) {
        el->x = newData;
        if (list->first == NULL) {
            el->last = NULL;
            el->first = NULL;
            list->first = el;
            list->last = el;
        } else {
            el->last = NULL;
            el->first = list->last;
            list->last->last = el;
            list->last = el;
        }
        ++list->size;
        return;
    }
    for (int i = 0; i < position; i++) {
        el = el->last;
    }
    sk->first = el->first;
    ListNode before = list->first;
    for (int i = 0; i < position - 1; i++) {
        before = before->last;
    }

    sk->last = before->last;
    before->last = sk;
    sk->last->first = sk;
}

int main() {
    ListMaster list = malloc(sizeof(ListMaster));
    list->first = NULL;
    list->last = NULL;
    list->size = 0;
    push(5, list);
    push(6, list);
    push(7, list);

    push_on_position(1, 1, list);
    push_on_position(1, 1, list);

    //Need to be fixed
    //pop_position(1, list);
    print(list);

    //implement destroy - free all nodes and master
    //destroy(list)

    return (0);
}

output

Note [Is it a good idea to typedef pointers](https://stackoverflow.com/questions/750178/) — the succinct answer is "No". I'm not sure whether it's better or worse when the typedefs aren't actually used. — Jonathan Leffler, May 27 '18 at 14:31
@dennis-vash HeY! Thank you It's working. But honestly I want to understand why my pushonposition function wasn't working too. I've tried to debug and there's problem with line `el->x = newData;` and it's causing segmentation fault error but i don't know why. I've tried to rewrite my function using your code because it's working but when i see almost 100% similarity in codes I still get segmentation fault error in CodeBlocks... — Sz3jdii, May 27 '18 at 16:40
What I saw when was debugging your code is that you tried to access a field of a NULL pointer, in general I think you should try and debug on other IDE than CodeBlocks and follow the memory stack each step. @szeejdi — Dennis Vash, May 27 '18 at 16:56

Push on position and pop from position functions in double conected list

1 Answers1