C - linked list removing values on insertion-CodePudding

I'm trying to create a method that lets me insert a new node in my liked list at a chosen index, it is currently working as intended when inserting into index 0 or 1 but when I try to insert into any index >= 2 the first value in the list is being lost. any ideas why?

example main:

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


int main( void ) {
  List list = new_list();
  
  add(list, "Three");
  add(list, "Two");
  add(list, "Zero");

  print_list(list);

  printf("Inserting at 1 \n");
  insert_at(list, 1, "one")
  print_list(list);
  printf("Inserting at 2 \n");
  insert_at(list, 2, "inserted")
  print_list(list);

header file:

typedef struct Node{
    char *value;
    struct Node *next;
}Node;
typedef Node** List;
List new_list();
Node *new_node(char *value);
void add(List list,char *value);
int is_empty(List list);
void print_list(const List list);
int insert_at(List list,int index,char *value);

method file:

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


List new_list(){
    List list = malloc(sizeof(List));
    *list = NULL;
    return list;
}

Node *new_node(char *value){
    Node *node = malloc(sizeof(Node));
    node->value = value;
    node->next = NULL;
    return node;
}

void add(List list,char *value){
if (*list == NULL){
    *list = new_node(value);
}else {
    Node *node = new_node(value);
    node->next = *list;
    *list = node;
}
}

int is_empty(List list){
if (*list == NULL){
    return 1;
}   return 0;
}

void print_list(const List list){ 
    printf("[");
    Node *curr = *list;
if (curr == NULL){
    printf("]\n");
    return;
} 
    while (curr->next != NULL){
        printf("\"%s\", ", curr->value );
        
        curr = curr->next;
    }

    printf("\"%s\"", curr->value );
    printf("]\n");
}



int insert_at(List list,int index,char *value){
 
if ((index > 0 && is_empty(list) == 1) || index < 0){
    return 0;
}
    int i= 0;
 if (index == 0){
    add(list, value);
    return 1;
}
  
while((*list) != NULL){
    //advancing loop
    i  ; 
//checking if wanted index = lists index
if (i == index){
    //creating new node
    Node *node = new_node(value);
    //updating next values;
    node->next = (*list)->next;
    (*list)->next = node;
    
    return 1;
}
  (*list) =(*list)->next;
}
  return 0;
}

example output:

["Zero", "Two", "Three"]
Inserting at 1
["Zero", "One", "Two", "Three"]
Inserting at 2
["One", "INSERTED", "Two", "Three"]

CodePudding user response：

In this while loop

while((*list) != NULL){
    //advancing loop
    i  ; 
//checking if wanted index = lists index
if (i == index){
    //creating new node
    Node *node = new_node(value);
    //updating next values;
    node->next = (*list)->next;
    (*list)->next = node;
    
    return 1;
}
  (*list) =(*list)->next;
}

the statement

  (*list) =(*list)->next;

overwrites the pointer to the head node that results at least in numerous memory leaks.

And the second function parameter should have an unsigned integer type as for example size_t.

Also the function new_list should look at least like

List new_list(){
    List list = malloc(sizeof( *list ));
    *list = NULL;
    return list;
}

In general it is a bad approach with using such a typedef

typedef Node** List;

It only confuses readers of the code.