0

I am currently trying to create a phonebook with a linked list. I am focusing first on the insertion function but the problem is that once I create 3 - 5 nodes, onlineGDB shows the error "malloc : corrupted top size", and VS Code shows that I got a segmentation error.

I assume this is an error with the way I am allocating memory. This is the first time that I am working on a structure that contains strings as data instead of integer so I might have missed a thing or two.

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

struct node{
    char name[30];
    char number[15];
    struct node *next;
};

void showMenu();
void insertData(struct node **head, char person[], char phone[]);
void showData(struct node *head);

int main(){

    struct node *head = NULL;
    char name[30], number[15];
    while(1)
    {
        printf("Name : ");
        scanf(" %[^\n]s", name);
        printf("Number: ");
        scanf(" %[^\n]s", number);
        insertData(&head, name, number);
        showData(head);
    }
    return 0;
}

void insertData(struct node **head, char person[], char phone[]){

    struct node *new_node = (struct node*) malloc(sizeof(struct node*));
    strcpy(new_node->name, person);
    strcpy(new_node->number, phone);
    new_node->next = *head;
    *head = new_node;
}

void showData(struct node *head){

    while(head != NULL)
    {
        printf("%s\t\t%s\n", head->name, head-> number);
        head = head->next;
    }    
    printf("\n");
}
Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
blurridge
  • 3
  • 2
  • 3
    `malloc(sizeof(struct node*));` You want to store a `struct node` in that memory location, not a pointer to `struct node`. A pointer is only 4 or 8 bytes, way too small for your struct => `malloc(sizeof(struct node));` or `malloc(sizeof(*new_node));` – Gerhardh Feb 16 '22 at 14:07
  • 2
    Side note: the cast isn't needed with `malloc`: `(struct node*)malloc(...` -> `malloc(...` – Jabberwocky Feb 16 '22 at 14:13
  • Note that these scanf calls are [vulnerable to buffer overruns](https://stackoverflow.com/q/1621394/10077). – Fred Larson Feb 16 '22 at 14:17
  • @Gerhardh this worked. Thank you so much! So everytime I use malloc for linked lists, I always use the size of the structure itself? Not a pointer to the structure? – blurridge Feb 16 '22 at 14:17
  • 1
    `" %[^\n]s"` is not what you want. That matches (and consumes) all input up to the first newline. The next character is a newline, so the `s` is guaranteed not to match. The `[^\n]` is *not* some sort of modifier for a `%s` conversion specifier. Your format string is trying to match a literal `s`. Use `" %29[^\n]"` and `" %14[^\n]"` – William Pursell Feb 16 '22 at 14:19
  • 1
    _"So everytime I use malloc for linked lists, I always use the size of the structure itself? Not a pointer to the structure?"_: yes, obviously. Every time you allocate memory for a structure you need to allocate enough memory so you can store that structure. – Jabberwocky Feb 16 '22 at 14:20
  • @Jabberwocky one question, when do I need to cast malloc()? Been doing it out of habit for the past semester. – blurridge Feb 16 '22 at 14:20
  • 1
    @blurridge you never need to cast the return value of `malloc`. `malloc` returns a `void*` and `void*` are safely promoted to any pointer type. – Jabberwocky Feb 16 '22 at 14:22
  • @WilliamPursell I used that format to make sure that the buffer does not carry over to the next string. Will your formatting also make it work? – blurridge Feb 16 '22 at 14:22
  • 2
    If you use the pattern `x = malloc( n * sizeof *x )` you don't need to think about it. In your case, that would be `struct node *new_node = malloc(1 * sizeof *new_node);` (the `1*` here is unnecessary, but given to be explicit) – William Pursell Feb 16 '22 at 14:22

1 Answers1

0

Use malloc(sizeof(struct node)) instead of malloc(sizeof(struct node*)) in your function insertData(). You want to allocate the size of the node and not the pointer to node.

onapte
  • 217
  • 2
  • 8