0
int add_server(Server *list, char *server_id, char *capacity)
{
    Server *new_node=(Server *)malloc(sizeof(Server));

    new_node->id=server_id;
    new_node->capacity=atoi(capacity);

    if(list==NULL)
    {
        list=new_node;
        list->next=list;
        return list;
    }
    else
    {

        Server *temp=list;

        while(temp->next!=list)
        {
            temp=temp->next;
        }

        temp->next=new_node;
        new_node->next=list;
    }

}
cnicutar
  • 178,505
  • 25
  • 365
  • 392
user217895
  • 97
  • 5
  • 3
    Could you add some text to that code, describing the problem beyond the rather terse title ? – cnicutar Apr 03 '13 at 09:28
  • You don't allocate memory for `server_id` in `add_server`. Do you allocate memory in the calling function or keep updating the same char array? The latter will mean that all nodes point to the same `server_id` instance so will all have their ids updated. – simonc Apr 03 '13 at 09:33

1 Answers1

1

First, please don't cast malloc()'s return value in C.

Second, the return list; is strange in a function returning int, that might cause problems. Clearly the function must return Server * (the type of list) in order to make sense. Or it could make the list argument have type Server ** and re-write the caller's pointer, but that's often less convenient to use.

Third, you only copy the pointer for the id, so if calling this with e.g. a local char buffer holding different names, you will get the effect you describe. You need to make the id an actual array in Server, or call strdup() on the argument before storing.

So, either:

typedef struct {
    /* ... */
    char server_id[32];
    /* ... rest of fields ... */
} Server;

strlcpy(new_node->server_id, server_id, sizeof new_node->server_id);

or:

new_node->id = strdup(server_id);
Community
  • 1
  • 1
unwind
  • 391,730
  • 64
  • 469
  • 606