Basic C linked list syntax error

Question

Note: I already got the intended function to work with my own code, but I saw a tutorial on another website and am wondering why it doesn't work.

https://www.eskimo.com/~scs/cclass/int/sx8.html

The premise is as follows:

I'm playing around with a very basic linked list:

typedef struct node {
    int val;
    struct node * next;
} node_t;

I am trying to have a function remove an entry by value. It is as follows:

int remove_by_value(node_t ** head, int val) {
    for(head = &node_t; *head != NULL; head = &(*head)->next){
        if ((*head)->val == val) {
            *head = (*head)->next;
            break;
        }
    }
}

However, I'm getting an error when calling this function, namely:

"prog.c:35:17: error: expected expression before 'node_t'
 for(head = &node_t; *head != NULL; head = &(*head)->next){
 ^"

Any ideas? Is this just a simple syntax error that I'm not seeing? Thanks!

In addition to the syntax errors, your logic for splicing out an element is wrong. — Tim Biegeleisen
– Tim Biegeleisen, Commented Jan 13, 2016 at 23:42
so many things are wrong in this code, it's hard to pick where to start — cozyconemotel
– cozyconemotel, Commented Jan 13, 2016 at 23:48
I actually took this method off of a website I saw: eskimo.com/~scs/cclass/int/sx8.html I've gotten the code to work using a different approach, but am wondering why the approach shown in this website doesn't work. — riceman89
– riceman89, Commented Jan 13, 2016 at 23:56

user3629249 · Accepted Answer · 2016-01-14 00:20:03Z

2

the root of the problem is that node_t is a type, not a variable and cannot take the address of a type.

The following code cleanly compiles.

be sure to check the logic,

for first iteration of the loop when head = NULL or only one struct in linked list
check logic for when desired struct is either last or next to last in the linked list

here is the code:

typedef struct node
{
    int val;
    struct node * next;
} node_t;


int remove_by_value(node_t ** head, int val)
{
    int retVal = -1;  // initialize to failed
    node_t  *previousNode = *head;
    node_t  *currentNode = *head;

    for(;
        previousNode && currentNode; // assure something to test
        previousNode = currentNode,  // update the pointers
        currentNode = currentNode->next )
    {
        if (currentNode->val == val)
        {
            previousNode->next = currentNode->next;
            retVal = 0; // indicate success
            break;
        }
    }
    return retVal;
} // end function: remove_by_value

answered Jan 14, 2016 at 0:20

user3629249

16.6k1 gold badge18 silver badges17 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

riceman89 Over a year ago

Thanks a lot for the clear explanation. Didn't realize that I was trying to perform variable calls on a typedef. Very helpful! :)

gollum · Accepted Answer · 2016-01-15 11:51:00Z

1

Since I cannot comment on the accepted answer written by @user3629249: That code is even worse than the original (except that it would compile).

I'd suggest something like this:

node_t *remove_by_value(node_t **head, int val)
{
    node_t *ret = NULL;

    for (; *head; head = &((*head)->next))
    {
            if ((*head)->val == val)
            {
                    ret = *head;
                    *head = (*head)->next;
                    break;
            }
    }
    return ret;
}

This code correctly removes the element from the beginning, middle and end of the list. In addition it gives the caller the chance to free the unlinked node.

edited Jan 15, 2016 at 11:51

answered Jan 14, 2016 at 1:22

gollum

2,6121 gold badge20 silver badges22 bronze badges

6 Comments

riceman89 Over a year ago

What would you propose instead as a cleaner way of writing it? Thanks!

riceman89 Over a year ago

Thanks! This is a lot shorter, elegant, and works properly. Much appreciated!

user3629249 Over a year ago

I wrote my answer in way I did because: 1) avoided changing the OPs function signature, 2) did not pass the selected struct pointer to free() as the OP did not specify that the linked list consists of memory allocations from the malloc() family 3) this posted answer is changing the callers' pointer, even when the selected struct is NOT the first struct in the linked list.

gollum Over a year ago

@user3629249 nevertheless the result of your changes is e.g. that the first node cannot be deleted. There is a reason why the address of the listhead pointer is passed to the function.

user3629249 Over a year ago

I did say to check the algorithm for the edge cases (as it may not be correct)

|

Sijaan Hallak · Accepted Answer · 2016-01-14 00:21:39Z

0

1) The error you are getting was pointed out by iharob already. 2) I can understand that out of this head =&t_node you want head to point at the head of your list. a static variable may be needed in your file to be able to use this then you can point head correctly

answered Jan 14, 2016 at 0:21

Sijaan Hallak

1659 bronze badges

Collectives™ on Stack Overflow

Basic C linked list syntax error

3 Answers 3

1 Comment

6 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

6 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related