CODEWITHSUNDEEP
cstringmallocfree
Ariel Ariel
Ariel Ariel

Reputation: 13

free(); and malloc(); keeps crashing (C)

I built this code to practice pointers and the program keeps crashing.it seems it crashes when I enter a big number to counter. 1-5 doesn't affect it apparently, but when you enter 30 it keeps crashing, sometimes on the allocation itself malloc(... and sometime in the free(names[i]); function.

What's the problem here?

#include "stdafx.h"
#include <stdio.h>
#include <stdlib.h>
#include <malloc.h>
#include <string.h>


int main() {
    char **names;
    char buffer[100];
    int i, bufferLen, counter;

    printf("how many names? ");
    scanf_s("%d", &counter);
    if (counter < 0) {
        printf("wrong choice\n");
        return 1;
    }

    names = (char**)malloc(77 * sizeof(char));
    if (names == NULL) {
        printf("failed...\n");
        return 1;
    }

    for (i = 0; i < counter; i++) { 
        printf("write the name!! (up to 100 chars): \n");
        gets_s(buffer, sizeof(char) * 100);
        bufferLen = strlen(buffer) + 1;
        names[i] = (char*)malloc(sizeof(char)*bufferLen);
        if (names[i] == NULL) {
            printf("failed...\n");
            return 1;
        }
        strcpy_s(names[i], sizeof(char)*bufferLen, buffer);
    }

    for (i = counter-1; i >= 0; i--) { //print names
        printf("no. %d, ptr no. %d (size: %d bytes): \n", i+1, (int)(names[i]), sizeof(names[i]));
        puts(names[i]);
    }
    for (i = 0; i < counter; i++) { 
        if (names[i] != NULL)
            free(names[i]);
    }
    if (names != NULL)
        free(names);
    return 0;
}

Upvotes: 1

Views: 283

Answers (2)

Maxim Egorushkin
Maxim Egorushkin

Reputation: 136208

You probably want names = (char**)malloc(counter * sizeof(char*));.

Also free handles null pointers, no need to check the pointer for null before calling it.

Upvotes: 2

unwind
unwind

Reputation: 399753

This:

names = (char**)malloc(77 * sizeof(char));

is wrong, sizeof (char) is 1 which is not what you want.

It should be:

names = malloc(77 * sizeof *names);

This is the same as 77 * sizeof (char *) since names is char ** which makes the type of *names be char *.

The cast is not necessary and should be omitted in my opinion.

It's very strange (and an obvious code smell) to use a literal 77 instead of count for the array length, of course.

Upvotes: 4

Related Questions