linked list to get input from the user and print it

Question

I am trying a write a program in c to get an input (chars) from the user. The user should be able to type the input as long as he wants (''infinite'').

This is the program I finally wrote with getting no errors at all:

Code:

/*
main:

we will try to get an input from the user.
if we succeed, call insert function. 
if we don't succeed, call freeList function.
if we encounter '\n' or EOF, we quit the program.
print the list, the free it.

insert (type void. does not return a value):

if allocation is successful, then we can put a new item in the linked list. 
the item is a type node.
we don't care the number of nodes in the list. 
if allocation is not successful, call free function.

free:
free the memory.

*/

#include <stdio.h>
#include <stdlib.h>
typedef struct list *ptr;
typedef struct list{
    char data;
    ptr next;
}node;  /*describes a linked list.*/
void insert(ptr *H, char c);
void freeList(ptr *H);
void printList(ptr *H);

int main(){
char c;  
printf("enter a string\n");
ptr H=(ptr)malloc(sizeof(node));/*create an empty list. this is its head.*/
while ((c=getchar())!=EOF && c!='\n'){  
    insert(&H,c);
    }
printList(&H); /*print the list*/
freeList(&H); /*free the list*/
printf("\n");
return 0;
}
void insert(ptr *H, char c){
    ptr p1;
    p1=*H;
    ptr T=(ptr)malloc(sizeof(node)); /*try to allocate a new memory cell*/
    if (!T)
    {
        printList(H);       
        freeList(H); /*we should not free H because we will 
            lose the list we've created.*/
    }
    else
    {
        T->data=c;
        while(p1->next)
        {
            p1=p1->next;
        }
        p1->next=T; /*add T to the end of the linked list*/

    }


}
void freeList(ptr *H){
    ptr p1; /*helper pointer*/
    while(*H){      /*while H points to a certain node*/
    p1=*H;
    (*H)=p1->next;
    free(p1);
    }   
}
void printList(ptr *H){ /*a copy of H is sent so no problem working with it.*/
    ptr p1=*H; printf("string is: \n");
    while (p1) /*while H is not null        */
    {
        printf("%c", p1->data);
        p1=p1->next;
    }

}

This code actually works, but any feedback is good.

Answer 1

Attempting to get the address of a NULL pointer could cause the segmentation fault - you should make the HEAD of the list an allocated node rather than a NULL pointer.

It can also be useful to run code through a debugger such as gdb, which will tell you the line caused the segmentation fault (and also show the call stack).

Solving the warnings should be fairly simple.

• flag is unused so it can be removed.
• you should pass *H instead of H to printList
• you should pass H instead of &H to freeList

Alternatively, you could change your functions to take ptrs instead of ptr *s as I see no reason to pass node **s (which is what ptr * would become). In which case it should be a simple case of passing the Hs to the functions without worrying about their pointer type (although, as WhozCraig says, this might not be a good idea).

Answer 2

This is likely what you're looking for, with comments in-code to explain what is going on. Using pointers and more importantly, pointer-to-pointer logic is... unusual... the first time you are exposed to it. Hopefully this helps.

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

typedef struct list *ptr;
typedef struct list
{
    char data;
    ptr next;
} node;

void insert(ptr *H, char c);
void freeList(ptr *H);
void printList(ptr H);

int main()
{
    ptr H = NULL;
    int c;

    printf("enter a string\n");

    while (((c=getchar())!=EOF) && c!='\n')
        insert(&H,(char)c);

    printList(H); /*print the list*/
    freeList(&H); /*free the list*/
    printf("\n");
    return 0;
}

void insert(ptr *H, char c)
{
    // NOTE: while the pointer held in the pointer-to-pointer H
    //  is not NULL, move to the next pointer. Notice how we store
    //  the *address* of the 'next' member in our H variable as we
    //  walk. When we reach the end-of-list the value of the address
    //  held in the pointer whos address we're storing in H will be
    //  null. As a bonus, H will hold the address of the pointer we
    //  need to update, which we do.
    while (*H)
        H = &(*H)->next;

    // allocate new node, assigning to the pointer
    //  dereferenced by the address held in *H
    *H = malloc(sizeof(**H));
    (*H)->data = c;
    (*H)->next = NULL;
}

void freeList(ptr *H)
{
    // NOTE: same logic as before. using H as a pointer-to-pointer
    //  to walk through the chain of pointers. each time we save the
    //  value to a temp, advance to the next pointer, and delete the
    //  saved value.
    while (*H)
    {
        ptr tmp = *H;
        *H = (*H)->next;
        free(tmp);
    }
}

void printList(ptr H)
{
    // NOTE: No need for a pointer-to-pointer here.
    while (H)
    {
        printf("%c", H->data);
        H=H->next;
    }
}

All of that said, I strongly advise eliminating the ptr ideology entirely and simply using properly declared pointers with asterisks. C programmers want to see those. They call out, saying "Look! I'm a pointer" =O

Answer 3

a recursive freeList function could be

void freeList(ptr *H)
{  
   if(*H!=NULL)
   {
    freeList(&(*H)->next);
    free(*H);
   }
}

similarly printList and insert can be rewritten