Head constantly changing in c linked list

Question

I'm trying to implement a simple linked list in C, but it seems that the first element is not properly added. Here is my code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <strings.h>
#include <unistd.h>

typedef struct person{
    int phoneNumber;
    char* name;
    struct person *nextPers;
}person;

person* firstPerson=NULL;
person* lastPerson=NULL;

void addPerson (person* _person){
/*Adds a new person to the linked list after the last one added
*/  
if (firstPerson!=NULL){
    fprintf(stderr,"DEBUG(addPerson): First person is %s \n",firstPerson->name);
    fprintf(stderr,"DEBUG (addPerson): Last Person is (before adding the new one) %s \n",lastPerson->name);}
fprintf(stderr,"DEBUG: Adding person %s \n",_person->name);
_person->nextPers= NULL;
if (firstPerson==NULL){
    firstPerson = _person;
    lastPerson= _person;
    fprintf(stderr,"DEBUG: The head of the list is %s \n",firstPerson->name);   

}else{
    fprintf(stderr,"DEBUG: Last person (before adding the new one) %s \n",lastPerson->name);
    fprintf(stderr,"DEBUG (addPerson):Adding to the list %s \n",_person->name); 
    lastPerson->nextPers =_person;
    lastPerson=_person;
    fprintf(stderr,"DEBUG: Last person %s \n",lastPerson->name);
    }
}


int main(int argc, char* argv[]){
    char line[80],name[80];
    int number;
    setvbuf(stdout,(char*)malloc(sizeof(char)*80),_IOLBF,80);
    setvbuf(stdin,(char*)malloc(sizeof(char)*80),_IOLBF,80);

    for(;fgets(line,80,stdin);){
        if(!strcmp(line,"Finish\n"))
              break;

        sscanf(line,"%[^:]: %d",name,&number);
        /* Stores the person introduced from stdin as Name:phone */
        if(firstPerson!=NULL){
            fprintf(stderr,"DEBUG (Before storing new data aparently): First person is %s \n",firstPerson>name);    
        }
        person * newPerson= malloc(sizeof(person));

        newPerson->phoneNumber = number;    
        newPerson->name = name;

        fprintf(stderr,"DEBUG: Adding person %s \n",newPerson->name);
        fprintf(stderr,"DEBUG: phone number %d \n",newPerson->phoneNumber); 

        addPerson(newPerson);

        fprintf(stderr,"DEBUG: There is a new person on the list\n");                   

        }
}

The expected output is:

Harvey:12345
DEBUG: Adding person Harvey 
DEBUG: phone number 12345 
DEBUG: Adding person Harvey 
DEBUG: The head of the list is Harvey 
DEBUG: There is a new person on the list
Adam:23456 
DEBUG (Before storing new data aparently): First person is Harvey 
DEBUG: Adding person Adam 
DEBUG: phone number 23456 
DEBUG(addPerson): First person is Harvey
DEBUG (addPerson): Last Person is (before adding the new one) Harvey 
DEBUG: Adding person Adam 
DEBUG: Last person (before adding the new one) Harvey
DEBUG (addPerson):Adding to the list Adam 
DEBUG: Last person Adam  
DEBUG: There is a new person on the list

But instead, the output is:

Harvey:12345
DEBUG: Adding person Harvey 
DEBUG: phone number 12345 
DEBUG: Adding person Harvey 
DEBUG: The head of the list is Harvey 
DEBUG: There is a new person on the list
Adam:23456
DEBUG (Before storing new data aparently): First person is (null) //Wrong, its Harvey
DEBUG: Adding person Adam 
DEBUG: phone number 23456 
DEBUG(addPerson): First person is Adam //Wrong, its Harvey
DEBUG (addPerson): Last Person is (before adding the new one) Adam //Wrong, its Harvey
DEBUG: Adding person Adam 
DEBUG: Last person (before adding the new one) Adam //Wrong its Harvey
DEBUG (addPerson):Adding to the list Adam 
DEBUG: Last person Adam 
DEBUG: There is a new person on the list

to sum up, the main idea is to get a linked list that is: Harvey->Adam->(next) but I don't know where the error may be.

fprintf(stderr, "DEBUG (Before storing new data aparently): First person is %s \n", firstPerson>name); -> fprintf(stderr, "DEBUG (Before storing new data aparently): First person is %s \n", firstPerson->name); — Jabberwocky
– Jabberwocky, Commented May 29, 2017 at 9:26
And watch your compiler warnings: lastPerson->nextPers = _person;: here the type of _person is person* whereas the type of lastPerson->nextPers is struct proceso *. You mixed up something. — Jabberwocky
– Jabberwocky, Commented May 29, 2017 at 9:29
You show us the expected and actual output which is good, but you should also show the input. — Jabberwocky
– Jabberwocky, Commented May 29, 2017 at 9:30
And last but not least: instead of doing "printf debugging", learn how to use your debugger. It's far more efficient, and the time you spent learning will quickly pay off. — Jabberwocky
– Jabberwocky, Commented May 29, 2017 at 9:32

Tomaz Canabrava · Accepted Answer · 2017-05-29 09:39:02Z

The linked list part of your code is correct, and this can make you mad, I know. There are a few other errors in your code, nothing related to the linked list.

            fprintf(stderr,"DEBUG (Before storing new data aparently): First person is %s \n",firstPerson>name);

see that you wrote firstPerson>name instead of firstPerson->name, this is doing a boolean comparisson and returning zero - that's why you have a "null" there.

The second error is here:

    newPerson->phoneNumber = number;    
    newPerson->name = name

you are setting the phoneNumber to the pointer of the name and number arrays. So when you do your second print here:

DEBUG (addPerson): Last Person is (before adding the new one) Adam //Wrong, its Harvey

it's actually retrieving the correct value - because you overrode Harvey to Adam.

The fix is simple: copy the string into the newPerson instead of pointing it to the name.

newPerson->name = malloc(sizeof(char) * strlen(name) + 1); 
strcpy(newPerson->name, name);

and this should work and fix your issues.

Riccardo Bonafede · Accepted Answer · 2017-05-29 09:40:35Z

1

You have made some errors:

struct proceso *nextPers;

This make non sense, change to

person *nextPers;

then the name saved in your struct is pointing to name[80] ( the var in main()), so every time your this var change, also the one in your struct change. You have to copy it on your struct;

memcpy(newPerson->name, name, 80);

and change your struct as:

typedef struct person{
    int phoneNumber;
    char name[80];
    struct person *nextPers;
}person;

Remember also to free al the node in your list when you finish

edited May 29, 2017 at 9:40

answered May 29, 2017 at 9:34

Riccardo Bonafede

6061 gold badge9 silver badges17 bronze badges

Collectives™ on Stack Overflow

Head constantly changing in c linked list

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related