Segmentation Fault in a function based on a linked list

Question

Writing a function to add a grade to the end of a linked list. There is a linked list of students, each student containing a pointer to a linked list of grades-

typedef struct _grade {
    char name[10];
    double value;
    struct _grade *next;
} Grade;

////////////////////////////////////////////////////////////////////////////////////////

typedef struct _student {
    char *lastName;
    char *firstName;
    Grade *headGradeList;
    struct _student *next;
} Student;

When I run my code after compiling, I get a segmentation fault. Im pretty sure it occurs in my if statement with the strcmp. Any suggestions?

// add a grade to the specified student
//   1. Make sure a student by that name exists (so you can add grade to it)
//   2. If the specifed grade already exists, update the grade's value to the new value
//   3. Otherwise, add the grade to the end of the student's grade list (add-at-end)

void addGrade(Student *headStudentList, char last[], char first[], char gradeName[], double value) {

    int flag=0;
    Student *dummy=headStudentList;
    Grade *temp=malloc(sizeof(Grade));
    strcpy(temp->name,gradeName);
    temp->value=value;
    temp->next=NULL;

    while(dummy!=NULL){
        printf("Here 1");
        if(strcmp(dummy->lastName, last)==0 && strcmp(dummy->firstName, first)==0){
            flag=1;
            if(dummy->headGradeList==NULL){
                strcpy(dummy->headGradeList->name, gradeName);
                dummy->headGradeList->value=value;
                dummy->headGradeList->next=NULL;

        }
        else{
            while(1){
                if(dummy->headGradeList->next==NULL){
                    dummy->headGradeList->next=temp;
                    break;
                }
                dummy->headGradeList=dummy->headGradeList->next;
            }
        }}

        dummy=dummy->next;
    }
    if(flag==0){
        printf("ERROR: student does not exist\n");
    }
}

You shouldn't use underscores as the first characters of your struct tags. — S.S. Anne
– S.S. Anne, Commented Nov 21, 2019 at 20:37
@JL2210 my instructor made the structs, the function is the only code I'm allowed to manipulate — CHuck
– CHuck, Commented Nov 21, 2019 at 20:39
Well, pass that on to your instructor. IIRC identifiers beginning with an underscore are reserved to the implementation. — S.S. Anne
– S.S. Anne, Commented Nov 21, 2019 at 20:41
"hey teacher, I was trying to get StackOverflow to complete my homework and they said..." — infixed
– infixed, Commented Nov 21, 2019 at 20:44
@JL2210 -- isn't that exactly the convention the instructor is using -- don't change things starting with _? Seems right to me — Hogan
– Hogan, Commented Nov 21, 2019 at 20:45

Maxim Sagaydachny · Accepted Answer · 2019-11-22 05:12:33Z

3

you forgot to initialize grades list with newly created entry

if(dummy->headGradeList==NULL){
                strcpy(dummy->headGradeList->name, gradeName);
                dummy->headGradeList->value=value;
                dummy->headGradeList->next=NULL;
            }

it should be:

if(dummy->headGradeList==NULL){
                dummy->headGradeList=temp;
            }

dummy->headGradeList in your case points to 0 (NULL) address.

dummy->headGradeList->name calculates into 0 as well - code expects your grade record to be found at 0 with name field being there as well since it has offest 0 from the start of the struct.

So your code essentially passes 0 as an address into strcpy function which tries to copy data into buffer located at address 0.

in most cases there is nothing there and this virtual memory is not mapped into any physical memory.

so this is the reason for the crash.

Also you have 3 more errors in the code:

1) else block should belong to grade list check (not student names)

2) check for existing grade (most likely you need to overwrite value for the grade which was added into the list earlier)

3) you are damaging (reseting grades list while you go through it). You need to use separate new variable which will be used as a pointer to the current grade in the list. you are using your pointer to the head to navigate through the list (i.e. you are modifying head while it should be immutable unless you decide to insert grade instead adding it to the tail)

Grade *current = dummy->headGradeList; // point iterator variable to head
//go to tail
while (current->next){
// here you need to check grade name and chenge existing grade. 
   current = current->next;
}
current->next=temp; // add new grade to the tail

alternatively you can insert grade into head (if you do not care about order and duplicates of grades)

if(we found right student){
  // it does not matter whether previous head pointed to NULL or good chain. just chain it back.
  temp->next=dummy->headGradeList;
  dummy->headGradeList=temp;
}

edited Nov 22, 2019 at 5:12

answered Nov 21, 2019 at 20:42

Maxim Sagaydachny

2,2483 gold badges15 silver badges26 bronze badges

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

2 Comments

Maxim Sagaydachny Over a year ago

thank you for reminder to take step back and look at things wider - i.e. from the percpective of student which studies pointers.

CHuck Over a year ago

@MaximSagaydachny Am I damaging in my else block?

Collectives™ on Stack Overflow

Segmentation Fault in a function based on a linked list

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related