Replace node value in a linked list

Question

I have a linked list and a setter function.

struct my_struct {
    int    value;
    int  type;
    char   *name;
    struct my_struct *next;
};

struct my_struct *setValue(struct my_struct *s, char *name, int b) {
    if(s!=NULL) {
        while(s != NULL) {
            if( strcmp(s->name,name) == 0) {
                s->value = b;
            }
            s=s->next;
        }
        return s;
    }
    return NULL;
}

Here, name is the search keyword and b is new value of s->value. Why s->value cannot change? After that function, the output is weird. I can't understand, what happened.

Why does this function return a pointer that is always NULL? What is it supposed to return? — wildplasser, Nov 24 '11 at 00:07

score 2 · Accepted Answer · edited May 23 '17 at 12:11

2

You need to be testing the strings equality with strcmp, as seen below. In your code, you're testing if two pointers are equal ^{[related post]}.

#include <string.h>

if(strcmp(s->name, name) == 0) { // if both strings are equal
    s->value = b;
}

The location of your return statement is interesting. You're returning the address of the last item that was changed, which may be undesired.

As per @Matthew Iselin's comment, change your loop to the following:

while(s != NULL) {
    ...
}

In case you are setting the root node to the return value of the function, s will always be NULL after looping through the linked list, therefore the function will always return NULL.

edited May 23 '17 at 12:11

Community

1
1

answered Nov 23 '11 at 23:21

I edited the question, according your answer about strcmp. But the problem, s->value cannot change still. – miqbal Nov 23 '11 at 23:40
I add four node in list that named p. Then I call setValue(struct my_struct *p, "name_here", new_value). Again I display the list. But just recently added is in list. And length of list is 1. – miqbal Nov 23 '11 at 23:50
let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/5291/discussion-between-moses-and-tim-cooper) – miqbal Nov 23 '11 at 23:58

score 1 · Answer 2 · answered Nov 23 '11 at 23:24

1

String comparison like Tim explained
You compare the next state in your while loop, and then you handle the current state. I.e. you can't change the value of your last entry in the list, since you abort the while loop before you set the new value.

answered Nov 23 '11 at 23:24

duedl0r

9,289
3
30
45

I don't understand second one. – miqbal Nov 23 '11 at 23:33
1

`s->next != NULL` should be `s != NULL`. At the moment, your loop traverses each node except the last one. – Matthew Iselin Nov 23 '11 at 23:40
@moses: after you set s->value, `break;` from the loop, and change the last return statement in the function to `return s;`. At the moment, you're returning NULL. – Matthew Iselin Nov 24 '11 at 00:54

RedComet · Answer 3 · 2011-11-24T01:13:13.030

You already got help about the loop, so I'll try whats is left.

Are you sure that both s->name and name have properly allocated memory? and that both are the same length and properly null terminated. The strcmp() function spects all this being true to give an equal response, and the allocated memory and null termination for any response at all.

If you have funny results, there is probably some memory leak somewhere. Try strncmp with a maximum character count to compare and see what happens.

Replace node value in a linked list

3 Answers3