C Simple LinkedList

Question

I have looked at many different questions online and cannot figure out what I am doing wrong. I may be headed in the wrong direction now because I have tried so many different things.

I am just trying to make a simple singly linkedList in C. I can not seem to figure out how to make the list stay connected.

The struct for my Node

typedef struct node
{
  double x;            // x-coordinate of this point in the tour
  double y;            // y-coordinate of this point in the tour
  struct node* next;   // Pointer to the next node in the linked list
} Node;

This is my code to make the list, I do construct an empty Node first = NULL in main

Node* addFront(Node* first, double x, double y) {   

     first = malloc(sizeof(Node));
     if (first == NULL) {
        first->x = x;
        first->y = y;
        first->next = NULL;
     }
     else {
        Node * temp = malloc(sizeof(Node));
        temp->x = x;        
        temp->y = y;                      
        temp->next = first;                 
        first = temp;     
     }
     //Temp testing
     int size = 0;
     Node * current = first;
     while (current->next != NULL) {
        printf("(%.4f, %.4f)\n", current->x, current->y);
        current = current -> next;
        size++;
     }
     printf("Size: %d\n", size);

  return first;
}

Some notes:

Checking to see if first is null should be unnecessary... The list should be able to be built just using the else statement. (What I think)

After adding the if/else statement I am getting what seems to be an infinite loop with C just pointing in random memory that eventually leads to segmentation fault.

I just do not know where else to turn to. Thank you so much in advanced!

Answer 1

This block doesn't make sense at all:

 first = malloc(sizeof(Node));
 if (first == NULL) {
    first->x = x;
    first->y = y;
    first->next = NULL;
 }

Probably you wanted to move the first = malloc(sizeof(Node)); inside the block. It would work, however it's completely unnecessary because it would be logically equal to the else block. So you can leave just the second block there:

    Node * temp = malloc(sizeof(Node));
    temp->x = x;        
    temp->y = y;                      
    temp->next = first;                 
    first = temp;
    return first;
    // or rather return temp directly

There is one more point - you should add error handling in case malloc runs out of memory, so you should check for temp == NULL and act accordingly (return NULL from function or whatever...).

Answer 2

For starters even the first statement of the function is wrong because the value of the parameter first is overwritten.

Node* addFront(Node* first, double x, double y) {   

     first = malloc(sizeof(Node));
     //...

The function can look the following way

Node * addFront( Node *first, double x, double y ) 
{   
    Node *temp = malloc( sizeof( Node ) );

    if ( temp != NULL )
    {
        temp->x = x;        
        temp->y = y;                      
        temp->next = first;                 

        first = temp;
    }

    return first;
}

Or with a testing code

Node * addFront( Node *first, double x, double y ) 
{   
    Node *temp = malloc( sizeof( Node ) );

    if ( temp != NULL )
    {
        temp->x = x;        
        temp->y = y;                      
        temp->next = first;                 

        first = temp;
    }

    // start pf inline test
    size_t size = 0;

    for ( Node *current = first; current != NULL; current = current->next )
    {
        printf( "(%.4f, %.4f)\n", current->x, current->y );
        ++size;
    }
    printf( "Size: %zu\n", size );
    // end pf inline test

    return first;
}