0

I have spent the last 2.5 hours creating this linked list and trying to understand why it is not passing a memory address to the head of the list. I'm trying to understand linked lists in C before I move onto what my class is learning about data structures in java. I looked into other questions and I don't understand why it doesn't work. Please excuse the comments I've been making an effort to understand everything. Thanks in advance for your time and help!

The head variable is equal to NULL after the new assignment head = addFamMember(NULL); in the main thread. But I can see that memory has been allocated inside the add function by printing it's members (name, age, and next pointer). Here's the output:

Enter command to add, print, or quit: add
Enter name and age: brett 28
Added:brett Age:28 POINTING to:(null) 


Enter command to add, print, or quit:

Here's the code: I left he comments to possibly help describe my thinking and pinpoint where I'm going wrong.

#include "stdio.h"
#include "stdlib.h"
#include "string.h"

typedef struct S_Family{
    char name[16];
    int age;
    struct S_Family *next;
}Family;

//prototypes
Family *addFamMember (Family *previous);
void CleanUp(Family *start);
void PrintList(Family *start);

int main(){
    setvbuf(stdout, NULL, _IONBF, 0);

    printf("Enter command to add, print, or quit: ");

    char input[16]; //fgets var to store input
    char command[16]; //sscanf var to store read info from input

    Family *head = NULL; //For a linked list we need to set up the first node and point it NULL
    Family *newest = NULL; //We also need a pointer dedicated to updating the latest created node

    //This while loop will continue to get input until the command "quit" is typed 
    //It includes the functionality of printing the list and adding new nodes
    while( fgets(input, 15, stdin)){
        sscanf(input, "%s", command);

        if ( strcmp(command, "quit") == 0) {
            printf("\n\nBreaking.........");
            break;

        } else if ( strcmp(command, "print") == 0) {
            PrintList(head);

        } else if ( strcmp(command, "add") == 0) {
            if ( head = NULL){
                head = addFamMember(NULL); //If there are no nodes give head a memory address so now we do (recursion somewhat?)
                printf("head:%s ", head->name); //this doesn't print!! Head = NULL above for some reason.
                newest = head; //newest cannot stay equal to NULL. this allows us to pass it as a param to add function w/out the start being NULL anymore              
                printf("newest:%s ", newest->name);
            } else {
                newest = addFamMember(newest); //Recursion where the new node gets a mem address and gets the previous node as guide to cont. the list
            }                                  //now we go to the add function
        }
        printf("\nEnter command to add, print, or quit: ");
    }

    CleanUp(head);

    return 0;
}

/*We want to return a new family member structure
so we start of with a Family return type. The param
as mentioned before needs to be a pointer to the address
of the previous node. That node will be pushed away from the
NULL in a singly or doubly linked list */
Family *addFamMember (Family *previous) { 


    /*Now we need to get the member variable info for that newFamMember
    and store into the newly created data structure newFamMember*/
    char input[16];

    printf("Enter name and age: ");

    fgets(input,15, stdin); 

    Family *newFamMember = malloc(sizeof(Family)); //create new address for newFamMember
    sscanf(input, "%s %d", newFamMember->name, &newFamMember->age); //takes the input (first a string then a integer) and stores it into its proper place

    printf("Added:%s Age:%d POINTING to:%S \n\n",newFamMember->name,newFamMember->age, newFamMember->next->name);

    newFamMember->next = NULL; //initialize it's pointer member var but more importantly maintains the linked list by pointing to null.

    /*Now we tell the computer what to do if this isn't the first node
    or not. If it is then there isn't a previous node so there is no
    way to set any other nodes' pointers to point to something else*/
    if ( previous != NULL){
        previous->next = newFamMember; //if previous is not equal to NULL set the previous' next pointer to newFamMember
        printf("previous:%s ", previous->next->name);
    }

    return newFamMember; //we always want to return a newly added family member. That's this function's purpose
}

/*now we can print the list*/
void PrintList (Family *head) { //start is a pointer so we can pass the value of start

    Family *currentMember = head; //we create currentMember and set it equal to start so we can iterate through the list and print each one

    int count = 0;
    if (currentMember == NULL){
        printf("There are no family members\n");
    }

    while (currentMember != NULL) {
        count++;
        printf("\n\nmember:%d Name:%s Age:%2d POINTING TO:%s\n",
                        count, currentMember->name, 
                        currentMember->age, 
                        currentMember->next->name);
        currentMember = currentMember->next; //move to the next node in the list headed towards NULL
    }
}

void CleanUp(Family *head){
    Family *freeMe = head;
    Family *holdMe = NULL;  
    while(freeMe != NULL) {
        holdMe = freeMe->next;
        printf("\nFree Name:%s Age:%d\n",
            freeMe->name,
            freeMe->age);
        free(freeMe);
        freeMe = holdMe;
        //PrintList(*start);
    }   
}
Sankofa
  • 600
  • 2
  • 7
  • 19
  • 3
    In the "add" section, that should very likely be `if (head == NULL)`, i.e. comparison rather than assignment. – M Oehm Jan 27 '15 at 14:54
  • Unfortunately that doesn't change anything. I tried that too. @MOehm – Sankofa Jan 27 '15 at 14:59
  • Really @Moehm ? You changed if (head == NULL) in the PrintList function? – Sankofa Jan 27 '15 at 15:06
  • OH MAN... smh I see. @Moehm – Sankofa Jan 27 '15 at 15:07
  • No, read my comment: In the "add" branch in `main`. Compiler warnings should tell you where exactly. You should also change the `%S` format to lower-case `%s`. And you are dereferencing a `NULL` pointer when printing the last entry for which `currentMember->next` is `NULL`. – M Oehm Jan 27 '15 at 15:08
  • Thank you very much @Moehm I really appreciate you taking a look at this. I was going crazy! – Sankofa Jan 27 '15 at 15:08
  • And the code prints `newMember->next` before it is set to NULL in the `add` code. It works for me, too, with the corrections suggested by @MOehm. On the whole, the `next` member should be printed with `%p` (pointer), and strictly, you should cast the address to `void *`. – Jonathan Leffler Jan 27 '15 at 15:09
  • I'm doing this in notepad++ so I don't get warnings... lesson learned. – Sankofa Jan 27 '15 at 15:10
  • 1
    Notepad++ is just the text editor. The warnings come from the compiler. – M Oehm Jan 27 '15 at 15:10
  • 1
    Notepad++ is an editor; it won't give you warnings. You are compiling with a compiler; that should be giving you warnings. Find out how to turn them on, then turn them on and pay attention to the warnings. Treat warnings as errors — the C compiler knows more about C than you do at this stage. – Jonathan Leffler Jan 27 '15 at 15:11
  • I will @jonathanLefflet because it didn't give me any warnings for this situation and just compiled. It sometimes does but not in this case. Thanks – Sankofa Jan 27 '15 at 15:14
  • should I cast it like this Family *newFamMember = (Family*)malloc(sizeof(Family) @jonathanLeffler or Family *newFamMember = (void *)malloc(sizeof(Family) – Sankofa Jan 27 '15 at 15:35
  • There is no point in a `(void *)` cast. If you have `#include ` at the top, the compiler knows `malloc()` returns `void *` anyway; if you don't have `#include ` at the top, it is not safe to use `malloc()` at all. There are those who object vehemently to `Family *newFamMember = (Family *)malloc(sizeof(Family));` — and in pure C, there is some justice on their side if you don't compile with options such that a missing `` generates errors (or warnings you treat as errors). On the other hand, if you're using a C++ compiler and the C subset of C++, it's necessary. – Jonathan Leffler Jan 27 '15 at 15:39
  • Why did you say, "you should cast the address to void * "? How do I do that? @jonathanLeffler – Sankofa Jan 27 '15 at 15:47
  • That was in the (not clearly spelled out) context of `printf("Added:%s Age:%d POINTING to:%S \n\n", newFamMember->name, newFamMember->age, newFamMember->next->name);` which needs fixing IMO to: ` printf("Added: %s Age: %d POINTING to: %p\n\n", newFamMember->name, newFamMember->age, (void *)newFamMember->next);` because you don't always have a valid pointer in `next`, and not all variants of `printf()` handle printing a string where the value passed is a null pointer. – Jonathan Leffler Jan 27 '15 at 15:53

0 Answers0