-3

So I am asked to develop a program that makes a family tree. As one of the first functions to do, is a function that finds if the input name has been already added to a person. my add function works fine but when I try to use the findPerson function, I get the error of Segmentation Fault (core dumped).

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


typedef struct _person {
    char *name;
    int birth;
    struct _person *nextperson;// pointer to the next person structure on
    struct _person *mother; //mother pointer to another person structure
    struct _person *father;//father pointer to another person structure

} person;

void addperson (person **families, char *name, int birthyear)
{
    person *newperson;
    if (strcmp(name,"unknown")!=0) {
        newperson=(person *)malloc(sizeof(person));//allocate space for the n
        if (newperson==NULL) {
            printf("insert: error: no space left\n");
            return;
        }
        //make space for newperson's name, the lenght of the input name
        newperson->name =(char *)malloc(sizeof(char)*(strlen(name) + 1));
        strcpy(newperson->name, name);//copy the input argument name to the
        newperson->birth=birthyear;
        newperson->nextperson=*families;
        newperson->mother=NULL;
        newperson->father=NULL;
        return;
    } else {
        printf("A person can not be called unknown\n");
    }

}

person *findperson (person *families, char *person)
{
    struct _person *finder= families;
    // finder =(person *)malloc(sizeof(person));
    //finder=families;
    if(strcmp(finder->name,person)==0) {
        return finder;
    } else {
        if (finder->nextperson!=NULL) {
            finder=finder->nextperson;
            findperson(finder, person);
        } else {
            printf("Person %s not found",person);
        }
    }
}

void main(){
    person *families= NULL; //initializ list with 0
    char*name,*name2;
    char command;
    person * foundPerson;
    int birth;
    int loop=1;
    while(loop==1){
        printf("command?");
        scanf(" %c", &command);

        switch(command){

            case 'q':
                printf("bye\n");
                loop=2;
                break;
                // case '\n':
                //  break;
            case 'i':
                printf("name? ");
                scanf(" %[^\n]s", name);
                printf("birthyear? ");
                scanf(" %d", &birth);
                addperson(&families, name, birth);
                break;
            case 'f':
                printf("name? ");
                scanf(" %[^\n]s", name2);
                foundPerson=  findperson(families,name2);
                printf("NAME FOUND: %s",foundPerson->name);
                break;
            default:
                break;


        }
    }
}
Nick S
  • 1,299
  • 1
  • 11
  • 23
Omar Caceres
  • 27
  • 1
  • 1
  • 3
  • 3
    Your `addperson` function doesn't really add anything. – Some programmer dude Oct 15 '18 at 13:01
  • 1
    Can you please format your code properly? Some of your comments are not comments, because they lack `/* ... */` or `//` – hellow Oct 15 '18 at 13:02
  • the add function adds the last element to the front of the database – Omar Caceres Oct 15 '18 at 13:04
  • i am editing the address of the families so I guess i am editing it with a new person – Omar Caceres Oct 15 '18 at 13:04
  • 1
    Yes and no. It creates a new item to add to the front of the database, but it doesn't actually update the database pointer. – Rup Oct 15 '18 at 13:04
  • 1
    And that's the reason for your crash, you're dereferencing a null pointer (which should have been very clear if you catched the crash in a debugger). You *always* need to check for null pointers. – Some programmer dude Oct 15 '18 at 13:06
  • 1
    so should I add one line saying * families=newPerson so that now the database points to the new person created – Omar Caceres Oct 15 '18 at 13:07
  • 1
    "so should I add one line saying *families=newPerson"? Yes, you should. You should also check for an empty list in `findperson` and return a pointer to the person if found, or NULL if not found. You do not need to use recursion. Iteration is safer, especially for a long list. You should also check the result of `findperson` is non-NULL before printing the name. – Ian Abbott Oct 15 '18 at 13:50

1 Answers1

1

This

     scanf(..., name);

scans input to where name points to. name is not initialised to point anywhere, so the scanner scans into invalid memory, and by doing so invokes undefined behaviour. Anything can happen from then on. The program might crash or not, immediately or later.

The same applies to this statement

    scanf(..., name2);

More about this unfortunately seemingly common mistake here.


Aside of the above issue this " %[^\n]s" does not what you expect. As it reads a string and then waits for an s.

It should just be " %[^\n]".

alk
  • 69,737
  • 10
  • 105
  • 255