adding a node to a linked list using a function

Question

I currently have a linked list and need to add data to it that is inputted by the user from the keyboard so i have two structs:

struct CourseInfo {
    int courseID;
    char courseName[30];
};
typedef struct CourseInfo courseinfo;
struct StudentInfo {
    char StudentID[10];
    char FirstName[21];
    char LastName[26];
    int num_course;
    courseinfo array[10];
    struct StudentInfo *next;
};

So i have a linked list with 3 nodes currently. I then need to call a function and add a node. The node needs to be inserted in the correct place which is that the studentID before it needs to be less than it and the studentID after needs to be greater so the current IDs i have are 111111111, 333333333, and 444444444 and im trying to add 222222222 so it would go in the second spot so my function looks like:

studentinfo *addStudent(studentinfo *data) //returns type studentinfo* now
{
    studentinfo *add;
    add = malloc(sizeof(studentinfo));
    add->next = NULL; //Now its set to NULL to begin
    int knt;
    printf("%s", "Adding new student:\nStudent ID: ");
    scanf("%s", add->StudentID);
    printf("%s", "First Name: ");
    scanf("%s", add->FirstName);
    printf("%s", "Last Name: ");
    scanf("%s", add->LastName);
    printf("%s", "Number of courses: ");
    scanf("%d", &add->num_course);
    for(knt = 0; knt < add->num_course; knt++) {
        printf("%s", "Course ID: ");
        scanf("%d", &add->array[knt].courseID);
        printf("%s", "Course Name: ");
        scanf("%s", add->array[knt].courseName);
    }
    if(searchStudentID(data, add->StudentID)) {
        puts("immediately inside if");
        while(data != NULL) {
            puts("Immediately inside while");
            if(strcmp(add->StudentID, data->StudentID) < 0) {
                puts("inside if");
                add->next = data;
                data = add;
            }
            else {
                puts("inside first else");
                studentinfo *PrevPtr = data;
                studentinfo *NPtr = data->next;
                while(NPtr != NULL) {
                    ("inside while(NPTR != NULL)");
                    if(strcmp(add->StudentID, NPtr->StudentID) < 0) {
                        add->next = PrevPtr;
                        PrevPtr->next = add;
                        break;
                    }
                    else {
                        puts("inside a differnet else");
                        PrevPtr = NPtr;
                        NPtr = NPtr->next;
                    }
                }
                if(PrevPtr->next == NULL) {
                    puts("inside last if");
                    add->next = NULL;
                    PrevPtr->next = add;
                }
            }
        }
    }
    else {
        puts("Found id");
    }
    return data; //returns data back to call
}

So i added all those puts statement because i wanted to see why the program kept crashing. So the puts statement puts("Inside a different else") is stuck in an infinite loop and keeps printing. The function searchStudentID simply returns 1 if we dont already have the ID and 0 if we already have it. I know that this function works so there is no need to post it.

I think the problem may be in the break; statement because it doesnt exit from the first while loop but only exits from the inner loop but im not positive.The call to this function look like:

list = addStudent(list); //Now the new data is stored in list

Where list is the linked list with 3 nodes

Answer 1

Linked list management is about managing node pointers, not just nodes. You want to do several things to make this considerably easier on yourself:

• Separate the input step from the search+insertion step. They don't belong together regardless of how they may seem otherwise. The biggest benefit this brings to you is reducing your list insertion code to what it should be doing (and only what it should be doing): managing the linked list. I've kept yours intact, but you should really be error checking and doing the data reading somewhere else.

• Use a pointer to pointer to walk the list. The biggest benefit from this is eliminating the need to special-case head-position insertion. If that is the position a new node will eventually occupy, so be it, but eliminating that special-case further reduces the complexity of the algorithm.

• Don't search the list unless you're capable of retaining the search results to be used for insertion logic. It makes little sense to perform an O(N) scan of the linked list to determine if input data is already present, only to search it again to find the position said-data will actually be inserted. Do it once. Find the position where it belongs. If it is already there, do nothing, otherwise, you sit at the precipice of the proper insertion location already.

• Finally, don't allocate a new node unless you know you need one. Use an automatic variable that helpfully self-discards if you end up doing nothing.

Putting all of that together gives something like this:

struct CourseInfo {
    int courseID;
    char courseName[30];
};
typedef struct CourseInfo CourseInfo;

struct StudentInfo {
    char StudentID[10];
    char FirstName[21];
    char LastName[26];
    int num_course;
    CourseInfo array[10];
    struct StudentInfo *next;
};
typedef struct StudentInfo StudentInfo;

StudentInfo *addStudent(StudentInfo *head)
{
    StudentInfo **pp = &head, *p = NULL, rec;
    int knt;

    // TODO: error check your inputs!
    printf("%s", "Adding new student:\nStudent ID: ");
    scanf("%s", rec.StudentID);
    printf("%s", "First Name: ");
    scanf("%s", rec.FirstName);
    printf("%s", "Last Name: ");
    scanf("%s", rec.LastName);
    printf("%s", "Number of courses: ");
    scanf("%d", &rec.num_course);
    for(knt = 0; knt < rec.num_course; knt++) {
        printf("%s", "Course ID: ");
        scanf("%d", &rec.array[knt].courseID);
        printf("%s", "Course Name: ");
        scanf("%s", rec.array[knt].courseName);
    }

    // walk the list pointers, starting with head, looking for
    //  a node that is equal or greater than the input node
    while (*pp && (knt = strcmp((*pp)->StudentID, rec.StudentID)) < 0)
        pp = &(*pp)->next;

    // leave now if already present
    if (*pp && knt == 0)
        return head;

    // allocate new node
    p = malloc(sizeof *p);
    if (p == NULL)
    {
        perror("Failed to allocate new node");
        exit(EXIT_FAILURE);
    }

    // structure copy.
    *p = rec;

    // link into proper list position.
    p->next = *pp;
    *pp = p;

    // always return the head (which may have updated above)
    return head;
}

That's it. As mentioned, I would personally perform the input operation somewhere other than this function, but I leave that to you to consider.

Best of luck.

Answer 2

Issues that I see:

1. You are not setting add->next to NULL.

2. You are changing data locally.

           add->next = data;
           data = add;
   

   changes the value of data locally in the function. It does not change the value in the calling function.

3. You have the check

   while(data != NULL)
   

   following the if statement

   if(searchStudentID(data, add->StudentID)) {
   

   but I don't see any code to add the new student when searchStudentID(data, add->StudentID) returns false and when data == NULL to start with.

Answer 3

Since this function updates the list you either need it to be

studentinfo *addStudent(studentifo *data)

and return the updated head value. Or

void addStudent(studentifo **data)

and do

*data = <new thing>