Why position 0 in delete node does not work?

Question

Im new to c programming. I wanted to create a linked list from a given file and then randomly get a node from linked list then delete that node. So the code works great but for the position 0 in linked list does not work. Please help me here's the code:

typedef struct node{
int *name;
struct node *next;
}node;

delete node:

void deleteNode(node **head_ref, int position){
if(*head_ref == NULL){
    return;
}
node * temp = *head_ref;
if(position == 0)
{
    *head_ref = (*head_ref)->next;
    return;
}
int h;
for(h=0 ; temp!=NULL && h<position-1 ; h++){
    temp = temp->next;
}
if(temp == NULL || temp->next == NULL)
    return;
node * next = temp->next->next;
free(temp->next);
temp->next = next;}

getting random node:

void RandomFromList(node *head){
// IF list is empty
if (head == NULL){
   return -1;
}
word = head->name;
// Iterate from the (k+1)th element to nth element
node *current = head;
int n;
for (n=2; current!=NULL; n++)
{
    // change result with probability 1/n
    if (rand() % n == 0)
       word = current->name;
    // Move to next node
    current = current->next;
}
sprintf(words , "%s" , word);
deleteNode(&head , search(head , word));
printf("Randomly selected key is %s\n", words);}

and the file Reader:

node* fileReader(FILE *file){
node *t = malloc(sizeof(node));
char TopicName[20];
int fileRead = fscanf(file,"%s",TopicName);
if(fileRead != EOF){
    t->name = strdup(TopicName);
    tedad++;
    t->next = fileReader(file);
}
if(fileRead == EOF) {
    return NULL;
}
return t;}

EDIT: When the code run's and when the position randomly got 0 the 0 position of linked list doesn't delete and continues with that node in linked list.

EDIT2:I changed my delete node and it works well without any problem, thank you guys!

node* deleteNode(node* head, unsigned i){
node* next;

if(head == NULL)
    return head;

next = head->next;

return i == 0
         ? (free(head), next)                              
         : (head->next = delete_at_index(next, i - 1), head);
}

Please be more specific on what does _not work_. Is there an error message? Does the actual result differ from the desired result? — Codor, Jan 25 '18 at 07:11
If "for the position 0 in linked list does not work", then you need to fix the code for position 0. With this I want to make obvious to you that your question is not clear enough. Please describe in more detail in which way it does not work. What unwanted thing happens? What wanted thing does not happen? — Yunnosch, Jan 25 '18 at 07:11
Please provide some debugging information. What are the values of all involved variable right before and after the attempt to do what is not working? Use printf()s to find out, or more advanced use a debugger: https://stackoverflow.com/questions/2069367/how-to-debug-using-gdb — Yunnosch, Jan 25 '18 at 07:13
With linked lists (and all other pointer problems), the solution usually gets much easier to find if you use a pen and paper, draw rectangles for variables, write their values inside. Use rectangles from which arrows point to other rectangles to represent pointers. Use the word "NULL" somewhere for your NULL pointers. Use a big "?" for uninitialised variables. ONLY change a variable if and when your program has a line for doing that. A pointer to a pointer is a rectangle from which an arrow points to a rectangle where an arrow starts. — Yunnosch, Jan 25 '18 at 07:15
why RandomFromList is a void function but returns something? — Parham Alvani, Jan 25 '18 at 11:08
can you please add your initiation function of your linked list nodes? if a node is the last node of list its next pointer point to NULL? — Parham Alvani, Jan 25 '18 at 11:13

Tim Biegeleisen · Accepted Answer · 2018-01-25T07:43:38.297

The major logical problem I see with your delete function is that it is void, i.e. it returns nothing. This is fine if the node being deleted is in the middle (or end) of the list, because the head does not change. But for the case of deleting the head, the caller might expect that his reference would then point to the next node (or null, if a list of one element) after making the call. Consider this code:

node* deleteNode (node *head_ref, int position)
{
    // passing in a NULL list returns NULL
    if (head_ref == NULL) {
    {
        return NULL;
    }

    // deleting the first element returns the second element as the new head
    node* temp = head_ref;
    if (position == 0)
    {
        node* ret = temp->next;
        free(head_ref);
        return ret;
    }

    // otherwise walk down the list to one before the deletion position
    for (int h=0; temp != NULL && h < position-1; h++) {
        temp = temp->next;
    }
    // if found, delete the node at the desired position
    if (temp != NULL && temp->next == NULL) {
        node* next = temp->next->next;
        free(temp->next);
        temp->next = next;
    }

    // for cases other than deleting the head, just return the current
    // (unmodified) head
    return head_ref;
}

I used your code like this , in RandomFromList: head = deleteNode(head , 0); it got logical error and I think it lost the linked list. — Ali Reza, Jan 25 '18 at 07:28
@Bob__ I used the changed code too but for the 0 position is link list it prints this when I want to print the list : http://bayanbox.ir/download/5585892716427685591/problem.png and it doesn't remove the other nodes too. — Ali Reza, Jan 25 '18 at 08:05

Stephen Docy · Answer 2 · 2018-01-25T09:45:20.187

0

This isn't related to your problem, but don't forget to free the memory:

node * temp = *head_ref;
if(position == 0)
{
    *head_ref = temp->next;
    free(temp);    // <--------
    return;
}

Also, you already have a pointer (temp) to *head_ref, it looks cleaner to me to just use that pointer instead of dereferencing head_ref again.

edited Jan 25 '18 at 09:45

answered Jan 25 '18 at 09:38

Stephen Docy

4,738
7
18
31

Parsa · Answer 3 · 2018-01-25T12:55:05.590

0

If you want to keep deleteNode void, then the problem is with your RandomFromList function. You are just changing the * head that exists in the function body not the pointer you passed to the function, so it's still pointing to the previous node.

It's because that pointers are passed by value (copied) like other things in C. Try making RandomFromList return the head pointer.

P.s. I think you also have some memory leaks in the delete function.

edited Jan 25 '18 at 12:55

answered Jan 25 '18 at 09:38

Parsa

1
2

I think this is not true, as he uses you can use pointer to pointer in order to change pointer value. for example, using `**head` you can change `head` pointer in your function but you must remember to call this function with `&head`. – Parham Alvani Jan 25 '18 at 11:26
He is using pointer to pointer in the delete method but not in the random method. Because he calls delete in the random function, I think he can't see the changes in where he called that function. – Parsa Jan 25 '18 at 12:43

joop · Answer 4 · 2018-01-25T10:29:19.547

0

void deleteNode(node **head_ref, int pos){
    node *del;

    for (   ; *head_ref; head_ref = &(*head_ref)->next) {
        if (pos-- <= 0) break;
        }

    if (!*head_ref) return; // Reached end of list: nothing found
    del = *head_ref;
    *head_ref = del->next;
    free(del);
    return;
    }

edited Jan 25 '18 at 10:29

answered Jan 25 '18 at 10:13

joop

4,330
1
15
26

Why position 0 in delete node does not work?

4 Answers4