CODEWITHSUNDEEP
clistpointersstructure
Mestralx
Mestralx

Reputation: 131

C - Popping last item from linked lists

I am learning linked lists and they are causing me a lot of troubles. I am calling the function with this call:

pop(&list);

ANd here's the code:

void pop(NODE** first) {
if(*first != NULL && first!= NULL){
    NODE* ptr = *first;
    while(ptr->next->next != NULL){
        ptr = ptr->next;
    }
    free(ptr->next);
    ptr->next = NULL;   
}

It's also causing memory leak error even if I call it single time..

When calling this function multiple times, there are more memory leak errors.

Thanks in advance, Mimpopo.

EDIT: Definition of NODE

typedef struct node {
    int data;
    struct node *next;
} NODE;

The full CODE:

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

typedef struct node {
    int data;
    struct node *next;
} NODE;



NODE* insert(NODE *first, int n){
    // create new node
    NODE* new = (NODE*)malloc(sizeof(NODE));
    new->data = n;
    new->next = NULL;

    // if first is NULL, this will be the first
    if(first == NULL)
        return new;

     // otherwise, place it correctly
     NODE* ptr = first;

     // check inserting at the begining
     if(ptr->data > new->data){
        new->next = ptr;
        return new;
     }

     // insert in the middle
     while(ptr->next != NULL){
        if(ptr->next->data > n && ptr->data < n){
            new->next = ptr->next;
            ptr->next = new;
            break;
        }
        ptr = ptr->next;
     }

     // insert at the end of list
     if(ptr->next == NULL){
         ptr->next = new;
     }

    return first;
}

void traverse(NODE *first){
    NODE* ptr = first;
    while(ptr != NULL){
        printf("%d\n", ptr->data);
        ptr = ptr->next;
    }   
}   

NODE* search(NODE *first, int n){
    NODE* ptr = first;
    while(ptr != NULL){
        if(ptr->data == n){
            printf("FOUND %d\n!", n);
            return ptr;
        }
    ptr = ptr->next;
    }
}

int main(){
    NODE* first = NULL;
    NODE* this = NULL;
    first = insert(first, 7);
    first = insert(first, 10);
    first = insert(first, 11);
    first = insert(first, 1);
    first = insert(first, 3);
    first = insert(first, 5);
    first = insert(first, 22);
    first = insert(first, 23);
    first = insert(first, 24);
    first = insert(first, 125);

    pop(&first);




}

Upvotes: 1

Views: 86

Answers (1)

Vlad from Moscow
Vlad from Moscow

Reputation: 311078

I have not looked through all your code but as for the function then it can be written the following way

void pop( NODE ** first ) 
{
    if ( *first != NULL )
    {
        NODE *prev = NULL;
        NODE *current = *first;

        while ( current->next ) 
        {
            prev = current;
            current = current->next;
        }

        if ( prev != NULL ) prev->next = current->next;
        else ( *first = NULL );

        free( current );
    }
}

As for your function implementation then it contains many errors. For example in this statement

if(*first != NULL && first!= NULL){

you shall swap the first and the second comparisons. That is the condition shall look like

if(first != NULL && *first!= NULL){

In this statement

while(ptr->next->next != NULL){

You have to be sure that ptr->nextis not equal to NULL.

Also you do not check whether the deleted node is the first node of the list.

Take into account that function insert is also wrong. You consider only one condition in this code snippet

 while(ptr->next != NULL){
    if(ptr->next->data > n && ptr->data < n){
        new->next = ptr->next;
        ptr->next = new;
        break;
    }
    ptr = ptr->next;
 }

However it can be that for example 

    ptr->next->data >= n && ptr->data < n

or

    ptr->next->data > n && ptr->data <= n

Upvotes: 1

Related Questions