C pointer to pointer skiss

Question

Is there anyone that can explain the following code. I have problem with double pointers and cant understand how is code is linked.

int remove_person(Post **list, char *name)
    {
        Post *p = *list;
        if(strcmp(p->name, name) == 0)
        {
            free(*list); //rensar minnet
            *list = p->next;
            return 1;  
        }
        for(; p->next != NULL; p = p->next)
        {
            if(strcmp(p->next->name, name) == 0)
            {
                Post *tmp = p->next;
                p->next = p->next->next;
                free(tmp); //rensar minnet 
                return 1;
            }
        }

        return 0;
    }

Read [Need of Pointer to pointer](http://stackoverflow.com/questions/18306935/need-of-pointer-to-pointer/18307020#18307020) — Grijesh Chauhan, Oct 02 '13 at 08:54
1 don't say it double pointer instead correct word is 'pointer to pointer' second you have undefined behavior (answered by wildplasser) — Grijesh Chauhan, Oct 02 '13 at 08:58

score 4 · Answer 1 · edited May 23 '17 at 12:13

4

There is nothing to understand. The code is wrong.

 // at this point *list and p _could_ be NULL

Post *p = *list;
  // At this point, *list and p point to the same object

if(strcmp(p->name, name) == 0)
    {  //   ^^^^--------------- WRONG: if p is NULL, dereferencing is not allowed

    free(*list); 
        // at this point the object has been destroyed

    *list = p->next; // .. and referenced AFTER ITS DESTRUCTION
           // ^^^^ <<------- WRONG
    return 1;  
    }

The *list = p->next; statement in the first if(strcmp(...) == 0) { } block references a pointer inside an object *list that has just been freed (p and *list point to the same object). The code is also overly complex. (please see my answer here for a correct (and simple!) way of doing it)

A simple fix to perform the operations in the correct order would be:

Post *p = *list;
if(p && strcmp(p->name, name) == 0)
    {
       *list = p->next;
       free(p); //rensar minnet
       return 1;  
    }

But a minimal verson, which does not contain special cases and does not repeat conditions and blocks of code would be:

int remove_person(Post **list, char *name)
{
    Post *del;
    for(; (del = *list); list = &(*list)->next) {
        if( strcmp(del->name, name) ) continue;

        *list = del->next;
        free(del); //rensar minnet 
        return 1;
    }

    return 0;
}

edited May 23 '17 at 12:13

Community

1
1

answered Oct 02 '13 at 08:34

wildplasser

43,142
8
66
109

This is good. Keeping the pointer-to-pointer in the loop, always pointing to the pointer to the current object, it's perfect. Just a thing, I recommend explicitly saying `if( strcmp(dell->name, name) != 0 ) continue;` because strcmp is too easily confused with something returning a Boolean value. – Medinoc Oct 02 '13 at 09:05
That is a matter of style. I prefer to omit the `!= 0`, since it is not needed (it is implied by the language's `int --> pseudo-boolean` promotion) And people who have used C longer than a week will **know** the behaviour of `strcmp()` – wildplasser Oct 02 '13 at 09:08
It's more a matter of readability than of need. I omit the `!= 0` when testing a variable that's supposed to hold a pseudo-Boolean value, but explicitly write it most other cases, *especially* `strcmp` because I've been burned more than once. Keep in mind the original poster is likely a beginner, likely to make this kind of mistake. – Medinoc Oct 02 '13 at 09:11
I am not a beginner and I prefer to write in my own style. Adding needless `!= 0` terms to an expression will only increase the amount of visual clutter. And personally, I've never been bitten by `strcmp()` There is only one value for zero, and it makes perfect sense to me to use that as the return value for `is_equal` because there is only one way of being equal (and more than one way of being different) – wildplasser Oct 02 '13 at 09:26
If you are going to program in C, you should understand its roots. In the flavor of C used by anybody writing highly portable code (C90), it has no booleans, so there is nothing to confuse the issue. – willus Oct 02 '13 at 11:40

score 2 · Answer 2 · answered Oct 02 '13 at 08:09

2

This is a rather simple use of pointer pointers: The function removes an element from a singly linked list, and able to modify the "head" pointer if the removed element was the first one.

answered Oct 02 '13 at 08:09

Medinoc

6,577
20
42

score 0 · Answer 3 · answered Oct 02 '13 at 08:12

Due to the fact that C is pass by value, in order for the modifications to the pointer passed as an argument to persist past the function scope, the author has decided to use the double pointer idiom.

If you were to pass list directly as a pointer, any modifications you do will not be visible outside of the scope of the function unless you were to return that pointer (and change the return type of the function).

But since int is returned to indicate success/failure, this is not an option

C pointer to pointer skiss

3 Answers3