CODEWITHSUNDEEP
cstring
JavascriptLoser
JavascriptLoser

Reputation: 1962

Call to malloc() causes unexplicable program crash

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.

Upvotes: 0

Views: 1046

Answers (2)

Sourav Ghosh
Sourav Ghosh

Reputation: 134326

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:

  1. Please do not cast the return value of malloc() and family in C.
  2. 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};

Upvotes: 3

Rohan
Rohan

Reputation: 53326

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.

Upvotes: 0

Related Questions