2

Can you help me to understand why this function doesn't delete a specific element in the linked list? What am I doing wrong?

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

...
node *head;
head = malloc(sizeof(node));
...

void delete_spec(node *a){
    int num;
    node *tmp;

    tmp = a;
    printf("Insert number to eliminate: ");
    scanf("%d",&num);
    while(tmp!=NULL){
        if(tmp->data == num){
            tmp = tmp->next->next;
        }
        tmp = tmp->next;
    }
}
Improve this question
7
  • 1
    Please make a minimal reproducible example. Commented May 7, 2020 at 14:03
  • 1
    Please state whether you try to delete a special case, like first or last in the list. Commented May 7, 2020 at 14:04
  • Try this method to analyse pointer problems stackoverflow.com/questions/59097696/… Commented May 7, 2020 at 14:05
  • @Boninissimo The provided code except the structure definition does not make sense. Commented May 7, 2020 at 14:07
  • 2
    @Boninissimo Don't edit the code as it invalidates the original question. Besides, it is still using tmp->next->next on the line after your edited line. Commented May 7, 2020 at 14:19

3 Answers 3

Reset to default
0

For starters it is unclear what the allocation is doing here

node *head;
head = malloc(sizeof(node));

The pointer head shall be initially set to NULL 

node *head = NULL;

and new nodes shall be inserted in the list by the function that inserts new values in the list.

The function that deletes nodes from the list shall not issue any prompt. It is the caller of the function that will ask the user to specify the value that will be deleted from the list and then call the function passing the specified value. So the function should have two parameters: pointer to the pointer to the head node and the integer value that shall be deleted from the list.

The function can be defined the following way

void delete_spec( node **head, int data )
{
    while ( *head != NULL )
    {
        if ( ( *head )->data == data )
        {
            node *tmp = *head;
            *head = ( *head )->next;
            free( tmp );
        }
        else
        {
            head = &( *head )->next;
        }
    }
}

Here is a demonstrative program.

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

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

int push_front( node **head, int data )
{
    node *new_node = malloc( sizeof( node ) );
    int success = new_node != NULL;

    if ( success )
    {
        new_node->data = data;
        new_node->next = *head;
        *head = new_node;
    }

    return success;
}

void delete_spec( node **head, int data )
{
    while ( *head != NULL )
    {
        if ( ( *head )->data == data )
        {
            node *tmp = *head;
            *head = ( *head )->next;
            free( tmp );
        }
        else
        {
            head = &( *head )->next;
        }
    }
}

void display( node *head )
{
    for ( ; head != NULL; head = head->next )
    {
        printf( "%d -> ", head->data );
    }

    puts( "null" );
}

int main(void) 
{
    node *head = NULL;
    int a[] = { 1, 3, 5, 7, 1, 2, 3, 1 };
    const size_t N = sizeof( a ) / sizeof( *a );

    for ( size_t i = 0; i < N; i++ )
    {
        push_front( &head, a[i] );
    }

    display( head );

    delete_spec( &head, 1 );

    display( head );

    return 0;
}

Its output is

1 -> 3 -> 2 -> 1 -> 7 -> 5 -> 3 -> 1 -> null
3 -> 2 -> 7 -> 5 -> 3 -> null
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

0 
public class LL5 {
Node head;
int size;
LL5(){
    this.size = 0;
}
class Node{
    String data;
    Node next;
    Node(String data){
        this.data = data;
        this.next = null;
        size++;
    }
}
public void push(String data) {
    Node newNode = new Node(data);
    if(head == null) {
        head = newNode;
        return;
        
    }
    newNode.next = head;
    head = newNode;
}
public void deleteelement(String data) {
    Node cur = head;
    Node x = cur.next;
    Node y = x.next;
    if(data == head.data) {
        head = head.next;
    }
    else if(x.data == data) {
        head.next = y;
    }
    else{
        while(y.data != data) {
            x = x.next;
            y = y.next;
        }
        x.next = y.next;    
    }   
}
public void printList() {
    if(head == null) {
        System.out.println("list is empty");
        return;
    }
    Node cn = head;
    while(cn != null) {
        System.out.print(cn.data + " -> ");
        cn = cn.next;
    }
    System.out.print("null");
}
public static void main(String[] args) {
    LL5 obj = new LL5();
    obj.push("1");
    obj.push("2");
    obj.push("3");
    obj.push("4");
    obj.push("5");
    obj.push("6");
    obj.push("7");
    obj.push("8");
    obj.push("9");
    obj.push("10");
    System.out.println("before delete");
    obj.printList();
    System.out.println();
    System.out.println("After delete");
    obj.deleteelement("10");
    obj.printList();
}

}

Improve this answer

1 Comment

As it’s currently written, your answer is unclear. Please edit to add additional details that will help others understand how this addresses the question asked. You can find more information on how to write good answers in the help center.
-1

delete_spec does not modify in anyway the input list. also: it does not free any memory.

in order to actually delete a node you must: 1. free its memory. 2. modify the list so the "next" pointers are updated. in order to update the list, you must provide the delete function the address of the head so it can modify also head.

something like this:

void delete_spec(node **a){
    int num;
    node *tmp;

    if (a == NULL || *a == NULL) return;

    tmp = *a;

    printf("Insert number to eliminate: ");
    scanf("%d",&num);

    if (tmp->data == num)
    {
        *a = (*a)->next;
        free(tmp);
        return;
    }


    while(tmp->next!=NULL){
        if(tmp->next->data == num){
            node* tmp2 = tmp->next;
            tmp->next = tmp->next->next;
            free(tmp2);
        }
        tmp = tmp->next;
    }
}
Improve this answer

7 Comments

tmp->next->next is not valid after free(tmp->next);.
Also, the while condition should be while(tmp->next!=NULL).
Also, *a = *a->next; should be *a = (*a)->next; or *a = tmp->next;.
@Boninissimo becsaue if the number to delete is in the head, you will not be able to modify the head pointer, unless you have a pointer to it, which is a pointer to a pointer.
@nivpeled The only other slight niggle is the function will crash if *a is NULL (for an empty list).
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.