Code giving runtime error?

Question

i am trying to solve leetcode question :-

https://leetcode.com/problems/largest-number/ Given a list of non negative integers, arrange them such that they form the largest number.

For example, given [3, 30, 34, 5, 9], the largest formed number is 9534330. i am trying to sort strings by defining comparator to compare strings by concatinating right-to-left or left-to-right.

The program is giving runtime error. Please help....

int comp(const void* a, const void* b){
        int p = *((int *)a);
        int q = *((int *)b);
        int size = 14;
        char * first = (char *)malloc(size * sizeof(char));
        char * second = (char *)malloc(size * sizeof(char));
        first[0] = "\0";
        second[0] = "\0";
        sprintf(first, "%d",p);
        sprintf(first, "%d",q);
        sprintf(second, "%d",q);
        sprintf(second, "%d",p);
        return -1*strcmp(first, second);
    }

    char* largestNumber(int* nums, int numsSize) {
        if(numsSize <=0)
            return NULL;
        qsort(nums, numsSize, sizeof(int), comp);
        char * result = (char*)malloc(numsSize *5*sizeof(char));
        int i;
        for(i=0; i<numsSize; i++)
            result = strcat(result, nums[i]);
        return result;
    }

You have to initialize the buffer pointed by `result` before passing it to `strcat()`. — MikeCAT, Nov 17 '15 at 14:40
You shouldn't do `first[0] = "\0";`, which is assigning an pointer to the element, whose type is `char`. Also, it is completely meaningless because it will be overwritten by `sprintf(first, "%d",q);`. `sprintf(first, "%d",p);` is useless, too. The same thing can by said for `second`. — MikeCAT, Nov 17 '15 at 14:43
What is the purpose of overwriting `first` and `second`? I mean, you first write `p` to `first`, then you write `q` to `first`, why? And if you have two integers, why compare them as strings? You can just do `return p - q;` (or `return q - p;` depending on order wanted). — Some programmer dude, Nov 17 '15 at 14:43
And are you sure you want to concatenate with `strcat` an array of `char` and an `int` — Manos Nikolaidis, Nov 17 '15 at 14:43

score 1 · Answer 1 · edited May 23 '17 at 11:59

Allocating memory with malloc() and throwing it away is a bad practice. Since you always allocate fixed amount of memory in comp, use regular array.
Don't do first[0] = "\0";, which is assigning an pointer to char variable. Also remove useless sprintf, whose result is soon be overwritten.
Allocating 5 bytes for each elements may be too small if int has 4 bytes. Allocate more memory.
Initialize the buffer pointed by result before passing it to strcat().
Convert the integer to string before passing it to strcat().
They say that you shouldn't cast the result of malloc() in C.

Possible fix (not tested):

int comp(const void* a, const void* b){
    int p = *((int *)a);
    int q = *((int *)b);
    char first[14];
    char second[14];
    sprintf(first, "%d", q);
    sprintf(second, "%d", p);
    return -1 * strcmp(first, second);
}

char* largestNumber(int* nums, int numsSize) {
    if(numsSize <= 0)
        return NULL;
    qsort(nums, numsSize, sizeof(int), comp);
    char * result = malloc(numsSize * 14 * sizeof(char));
    int i;
    result[0] = '\0';
    for(i = 0; i < numsSize; i++) {
        char num[14];
        sprintf(num, "%d", nums[i]);
        result = strcat(result, num);
    }
    return result;
}

Code giving runtime error?

1 Answers1