Structures/Linked Lists/Pointers/Strings in C

You need to keep track of the head pointer.

Here is the complete working code:

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


struct Student
{
    char * name;
    int idNum;
    char sex;
    struct Student * next;
};

struct Student * head=NULL;

struct Student *
insertNode (char *name, int idNum, char sex)
{

  struct Student *s;
  s = malloc (sizeof (struct Student));

  if (s == NULL)
    {
      printf ("Memory allocation failed.");
      return NULL;
    }

  (*s).name = name;
  (*s).idNum = idNum;
  (*s).sex = sex;


  (*s).next = head;     //head is the start of the list

  head = s;

  return head;          
}


int
main ()
{
    insertNode("Alice", 1, 'F');
    insertNode("Peter", 2, 'M');
    insertNode("Mike", 3, 'M');


  while (head != NULL)
    {
      printf ("%s\n", (*head).name);
      head = (*head).next;
    }

}

Output:

Mike
Peter
Alice

However, there are plenty of improvements that can still be done in your code. In particular, the notation (*s).name has a shorthand in C and that is s->name which is more clean. You can also avoid the global head variable and instead pass its pointer so that its changed value can be maintained in between calls.

(*s).name = name; sets the name in the struct to the local variable name which will become invalid at the end of the insertNode function.

You need to make a copy of the name.

You can use s->name = strdup(name); (Just rememeber you need to free s->name when you delete a node.

perhaps a simpler method whice you get your head around C would be to make name in the student node an array char name[32]; or similar. Then you would strcpy() into it but have one less thing to free.

Your question is a bit unclear as to the exact source of the problem. The answer by Syed does a good job identifying a solution, but there are some subtleties you need to be aware of as you are learning linked-lists.

First, your assignment of s->name = name; will only work if the names are string literals where you assign the address to the beginning of a string stored in read-only memory to s->name.

If you are reading values from a file (or stdin) and passing a pointer containing name to insertnode, then all of your nodes s->name will hold the pointer address, which, if it is still in scope will point to the storage containing the Last Name Read, and if your pointer used to pass name to insertnode has gone out of scope, you will invoke Undefined Behavior attempting to access s->name.

As correctly noted in the comments, the way to handle the situation is to allocate storage for each name passed to insertnode, assign the starting address to s->name and copy name to s->name. You can use strdup if you have that available, otherwise a simple allocation and strcpy is all that is required:

    size_t len = strlen (name);     /* get length of name */
    /* allocate/validate storage for name */
    if ((s->name = malloc (len + 1)) == NULL) {
        perror ("malloc - name");
        free (s);
        return NULL;
    }
    strcpy (s->name, name);         /*  copy name to s->name */

(note: since strdup allocates memory (e.g. in s->name = strdup(name);), you should still validate s->name is not NULL after the call. Also note that strings is C are terminated with the nul-characer, '\0'. Therefore, in order to allocate enough storage to hold name plus the nul-terminating character, you must allocate strlen (name) + 1 bytes.)

Next, while 100% OK, your insert-node-before head in a singly-linked list will have the affect of reversing the order of the nodes inserted into your list. While this may not matter for your implementation, it may be surprising to you when you do print the list. This does save iterating to the next free node on insert, but sacrifices input order to do so.

An alternative, would be to insert the nodes in order by simply checking if head is NULL, and if so, insert the first node, otherwise, iterate until list->next = NULL and insert the new node as list->next. (this also requires that you initialize all s->next nodes NULL when you allocate storage for them. A simple implementation (making use of a typedef for struct student for convenience), would look similar to:

/* returns pointer to new node on success, or NULL on failure */
student *insertnode (student **head, char *name, int idnum, char sex)
{
    student *s = malloc (sizeof *s);    /* allocate new node */

    if (s == NULL) {                /* validate allocation */
        perror ("malloc - s");
        return NULL;
    }

    /* populate new node */
    size_t len = strlen (name);     /* get length of name */
    /* allocate/validate storage for name */
    if ((s->name = malloc (len + 1)) == NULL) {
        perror ("malloc - name");
        free (s);
        return NULL;
    }
    strcpy (s->name, name);         /*  copy name to s->name */
    // s->name = name;      /* only works for string literals */
    s->idnum = idnum;
    s->sex = sex;
    s->next = NULL;         /* always initialize ->next NULL */

    if (*head == NULL) {            /* handle add 1st node */
        *head = s;
    }
    else {                          /* handle add rest */
        student *iter = *head;      /* declare pointer to head */
        while (iter->next != NULL)  /* while ->next not null */
            iter = iter->next;      /* get next node */
        iter->next = s;             /* set iter->next to new node */
    }

    return s;   /* head never changes, return current node 
                   to indicate success/failure of insert. */
}

Also, you should avoid declaring global variables. There is no reason your list shouldn't be declared in main() and a pointer (or address of the pointer if the list address could change) be passed as a parameter to any function that needs to operate on the list. Just move the declaration of head inside main(), and then add a pointer to pointer to list as a parameter for insertnode (as was done above).

You should free() all memory you allocate. (yes, this happens at exit, but building good habits now of preserving a pointer to the beginning address of all memory you allocate, and freeing that memory when it is no longer needed will pay dividends and your project grow in size.

Finally, when you iterate over your list, you must use a separate pointer. Otherwise, if you iterate using head, it is a one-way street. When you iterate until head == NULL, you have lost the only reference you had to all the memory you allocated and have no way of getting them back. Simply use a temporary pointer, e.g.

    student *iter = head;   /* use separate pointer to iterate list */
    while (iter != NULL) {
        printf ("%-8s  %4d  %c\n", iter->name, iter->idnum, iter->sex);
        iter = iter->next;
    }

(of course on your final trip over the list to free all the list memory -- it doesn't matter what you use -- there won't be any more list when you are done)

Putting it altogether, you could do something like the following:

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

typedef struct student {    /* typedef makes declarations easier    */
    struct student *next;   /* pointers 1st limits size to 24-bytes */
    char *name, sex;
    int idnum;
} student;

/* returns pointer to new node on success, or NULL on failure */
student *insertnode (student **head, char *name, int idnum, char sex)
{
    student *s = malloc (sizeof *s);    /* allocate new node */

    if (s == NULL) {                /* validate allocation */
        perror ("malloc - s");
        return NULL;
    }

    /* populate new node */
    size_t len = strlen (name);     /* get length of name */
    /* allocate/validate storage for name */
    if ((s->name = malloc (len + 1)) == NULL) {
        perror ("malloc - name");
        free (s);
        return NULL;
    }
    strcpy (s->name, name);         /*  copy name to s->name */
    // s->name = name;      /* only works for string literals */
    s->idnum = idnum;
    s->sex = sex;
    s->next = NULL;         /* always initialize ->next NULL */

    if (*head == NULL) {            /* handle add 1st node */
        *head = s;
    }
    else {                          /* handle add rest */
        student *iter = *head;      /* declare pointer to head */
        while (iter->next != NULL)  /* while ->next not null */
            iter = iter->next;      /* get next node */
        iter->next = s;             /* set iter->next to new node */
    }

    return s;   /* head never changes, return current node 
                   to indicate success/failure of insert. */
}

int main (void) {

    student *head = NULL;

    insertnode (&head, "Alice", 1000, 'F');    /* insert nodes */
    insertnode (&head, "Peter", 1001, 'M');
    insertnode (&head, "Mike",  1002, 'M');

    student *iter = head;   /* use separate pointer to iterate list */
    while (iter != NULL) {
        printf ("%-8s  %4d  %c\n", iter->name, iter->idnum, iter->sex);
        iter = iter->next;
    }

    /* free allocated memory */
    while (head != NULL) {  /* freeing list, pointer used doesn't matter */
        student *victim = head;     /* save pointer to node to delete */
        head = head->next;          /* move to next node */
        free (victim->name);        /* free storage for name */
        free (victim);              /* free storage for node */
    }
}

(note: while not an error, the standard coding style for C avoids the use of camelCase or MixedCase variable names in favor of all lower-case while reserving upper-case names for use with macros and constants. It is a matter of style -- so it is completely up to you, but failing to follow it can lead to the wrong first impression in some circles.)

Example Use/Output

$ ./bin/ll_head_next
Alice     1000  F
Peter     1001  M
Mike      1002  M

Memory Use/Error Check

In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.

It is imperative that you use a memory error checking program to insure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.

For Linux valgrind is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.

$ valgrind ./bin/ll_head_next
==30229== Memcheck, a memory error detector
==30229== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==30229== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==30229== Command: ./bin/ll_head_next
==30229==
Alice     1000  F
Peter     1001  M
Mike      1002  M
==30229==
==30229== HEAP SUMMARY:
==30229==     in use at exit: 0 bytes in 0 blocks
==30229==   total heap usage: 6 allocs, 6 frees, 89 bytes allocated
==30229==
==30229== All heap blocks were freed -- no leaks are possible
==30229==
==30229== For counts of detected and suppressed errors, rerun with: -v
==30229== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Always confirm that you have freed all memory you have allocated and that there are no memory errors.