Call to malloc() causes unexplicable program crash

Question

I'm having an issue where a call I'm making to malloc() causes my program to crash. Here's the code:

void update_item(char *input, item_t *new_node){
    int i, count, shelf, weight, price, quantity;
    char *name;
    char *specifier;
    char aisle[1];
    count = 0;

    /*Find name of the new item and assign to the name field of    new_node...*/
    for (i = 0; input[i] != ','; i++){
        count++;
    }
    name = (char*)malloc((count+1)*sizeof(char));
    if (name == NULL){
        printf("Out of memory. Shutting down.\n");
        exit(EXIT_FAILURE);
    }
    for (i = 0; input[i] != ','; i++){
        name[i] = input[i];
    }
    name[count+1] = '\0';
    new_node->name = name;
    printf("%s\n", new_node->name);

    /*Find aisle specifier and assign it to aisle field of new_node...*/
    i++;
    aisle[0] = input[i];
    aisle[1] = '\0';
    new_node->aisle = aisle;
    printf("%s\n", new_node->aisle);

    for(i = i+2, count = 0; input[i] != ','; i++){
        count++;
    }
    specifier = (char*)malloc(count*sizeof(char)); /*PROGRAM CRASHES HERE*/
    if (specifier == NULL){
        printf("Out of memory. Shutting down.\n");
        exit(EXIT_FAILURE);
    }
    printf("boom\n");

I'm utterly stumped. There's two identical calls to malloc() but for some reason the second fails every single time while the first one always is a success.

Describe "program crashes"? Does it print `"Out of memory. Shutting down."`? Do you have an error message? Have you tried a debugger? — Eregrith, May 12 '15 at 14:13
Standard Warning : Please [do not cast](http://stackoverflow.com/q/605845/2173917) the return value of `malloc()` and family in `C`. — Sourav Ghosh, May 12 '15 at 14:14
No it doesn't print my error message. Windows takes over and forces the program to close. — JavascriptLoser, May 12 '15 at 14:14
Also warning: when calling `sizeof` in `malloc` (and the like) [you should always write it](http://stackoverflow.com/a/17258659/1151654) as `ptr = malloc(sizeof(*ptr) * ...);` instead of `ptr = malloc(sizeof(ptrtype*) * ...);`. And you [should not cast](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) the return of malloc. — Eregrith, May 12 '15 at 14:14
What about all my other questions? Have you tried simply using a debugger? — Eregrith, May 12 '15 at 14:18
If you had used a debugger, you'd realize that the program doesn't crash at the malloc call, but later. — Lundin, May 12 '15 at 14:22
Read [this](http://stackoverflow.com/a/30191540/3049655) once again, please — Spikatrix, May 12 '15 at 14:25

score 3 · Accepted Answer · edited May 23 '17 at 11:44

Point 1

 for (i = 0; input[i] != ','; i++){

is unsafe. if your input does not contain , you'll be overrunning memory. Instead use something like

 int len = strlen(input);
 for (i = 0; (input[i] != ',') && len ; i++, len--){

Point 2

in C, we have 0 based index. so, for an allocation like,

name = malloc(count+1);

later, doing

name[count+1] = '\0';

is again meory overrun, which in turn invokes undefined behaviour.

Note:

Please do not cast the return value of malloc() and family in C.
sizeof(char) is guranteed to be 1 in C, you can get rid of that.

Point 3

as per your code, aisle is defined as

char aisle[1];

however, later, you used

aisle[0] = input[i];
aisle[1] = '\0';

which is again memory overrun and UB. Change you aisle to

char aisle[2] = {0};

score 0 · Answer 2 · answered May 12 '15 at 14:14

name[count+1] = '\0';

Above line should be

name[count] = '\0'; //one item less

Your code will cause write to 1 additional memory location which is not allocated by you.

There also problem with these lines ...

aisle[0] = input[i]; 
aisle[1] = '\0';    // aisle has only 1 char not 2
new_node->aisle = aisle; // you are storing reference of local  
                         // variable, not good.

Call to malloc() causes unexplicable program crash

2 Answers2