0

This is my code (working on a linked list):

typedef struct node {
   int data;
   struct node_t* next;
} node_t;

typedef struct list{
   node_t* head;
} list_t;

node_t* create_node(void){
   node_t* tmp = malloc(sizeof(node_t));
   if (tmp == NULL){
      fprintf(stderr, "Error while allocating memory for node\n");
      exit(EXIT_FAILURE);
   }
   return tmp;
}

void list_insert(list_t* list, node_t* node, int data){
   if (node == NULL){
      node_t* new = create_node();
      new->next = list->head;
      list->head = new;
      new->data = data;
   }
   else{
      node_t* current = list->head;
      while(current != node){
         current = current->next;
      }
      node_t* new = create_node();
      new->next = current->next;
      current->next = new;
      new->data = data;
   }
}

I do get some warnings in list_insert function, but I cannot figure out the reason for them. This function should insert a new node at the beginning, if the node passed as argument is NULL, otherwise it should insert a new node after the node passed as argument.

In this snippet:

if (node == NULL){
   node_t* new = create_node();
   new->next = list->head;
   list->head = new;
   new->data = data;
}

the assignment new->next = list->head; is not correct. Any guesses?

5
  • 1
    If you get warnings, please fix these first. If you need help with this, show in your question what warnings you get. Commented Feb 13, 2019 at 16:53
  • The loop to find node in the list should also handle the case that the end of the list is reached without finding node. Commented Feb 13, 2019 at 17:03
  • This is a duplicate many times over, bu the difficulty, as ever, is finding the right duplicate. I'm not sure whether there's a canonical Q&A for it. Typdefs, tagged and untagged structures, and incompatible pointer types is closely related, but not identical (that deals with presence/absence of tag, not directly dealing with misspelled tag causing confusion). Commented Feb 13, 2019 at 17:42
  • I put a corrected/modified version of your program if you are interested, with execution example Commented Feb 13, 2019 at 17:43
  • Enable Compiler Warnings, for gcc/clang, minimum -Wall -Wextra -pedantic, for VS /W3 and do not accept code until it compiles without warning. (you will save yourself untold hours just by following this rule) Let your compiler help you write better code. The compiler will tell you the exact line (and often the exact starting character) for the problematic code. Never ignore a warning. Commented Feb 13, 2019 at 17:59

2 Answers 2

1

In your definition of struct node:

typedef struct node {
        int data;
        struct node_t* next;
    } node_t;

You define next as a pointer to struct node_t, but there is no such type. You want struct node:

typedef struct node {
        int data;
        struct node* next;
    } node_t;
Sign up to request clarification or add additional context in comments.

Comments

1

you do not need typedef struct list{ ...}, you just need a variable of type node_t * always containing the head of the list, NULL while it is empty.

For instance :

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

typedef struct node {
  int data;
  struct node * next;
} node_t;

node_t* create_node(int data)
{
  node_t* tmp = malloc(sizeof(node_t));

  if (tmp == NULL){
    fprintf(stderr, "Error while allocating memory for node\n");
    exit(EXIT_FAILURE);
  }
  tmp->data = data;
  tmp->next = NULL;
  return tmp;
}

void list_append(node_t ** l, node_t* node){
  if (*l == NULL) {
    *l = node;
  }
  else {
    while ((*l)->next != NULL)
      l = &(*l)->next;

    (*l)->next = node;
  }
}

/* l, *l, where and node must not be NULL */
int list_insert(node_t ** l, node_t* where, node_t* node) {
  if (*l != where) {
    for (;;) {
      if (*l == NULL)
        return 0;
      l = &(*l)->next;
      if (*l = where)
        break;
    }
  }

  node->next = *l;
  *l = node;
  return 1;
}

void pr(node_t * l)
{
  printf("list is :");
  while (l != NULL) {
    printf(" %d", l->data);
    l = l->next;
  }
  putchar('\n');
}

void clear(node_t ** head)
{
  node_t * l = *head;

  while (l != NULL) {
    node_t * n = l->next;
    free(l);
    l = n;
  }

  *head = NULL;
}

int main()
{
  node_t * head = NULL;
  node_t * n3 = create_node(3);

  list_append(&head, create_node(1));
  list_append(&head, n3);
  pr(head);

  list_insert(&head, n3, create_node(2));
  pr(head);  

  list_insert(&head, head, create_node(0));
  pr(head);

  clear(&head);
  pr(head);

  return 0;
}

I changed the creation to be able to give the data at that time, this is more clear (like a constructor in C++)

Compilation and execution :

pi@raspberrypi:/tmp $ gcc -pedantic -Wextra c.c
pi@raspberrypi:/tmp $ ./a.out
list is : 1 3
list is : 1 2 3
list is : 0 1 2 3
list is :
pi@raspberrypi:/tmp $ valgrind ./a.out
==27963== Memcheck, a memory error detector
==27963== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==27963== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==27963== Command: ./a.out
==27963== 
list is : 1 3
list is : 1 2 3
list is : 0 1 2 3
list is :
==27963== 
==27963== HEAP SUMMARY:
==27963==     in use at exit: 0 bytes in 0 blocks
==27963==   total heap usage: 5 allocs, 5 frees, 1,056 bytes allocated
==27963== 
==27963== All heap blocks were freed -- no leaks are possible
==27963== 
==27963== For counts of detected and suppressed errors, rerun with: -v
==27963== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 6 from 3)

2 Comments

Very nice validation with valgrind.
valgrind is a fantastic tool, people has to know it exists, I put an execution under it the most I can.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.