an array for returning int in C programming-CodePudding

I have a question about return an array from a function in C, I do not know why it keeps giving the result as a code dumped. Please help, I really really thank you! I set up an int list3[size3] but it seems like I have to return with a format of an array as int*list3. So I set up another array as list4[size3] to copy list3 into list4. But I am not sure it is a reason it cause code dumped. If it is, please help me with the advice to solve the problem. Thank you so much again!.

#include<stdio.h>

int* merged(int[], int[], int, int);
void sort(int[], int);

int main() {

    int size1;
    printf("Enter list1: ");
    scanf("%d", &size1);

    int list1[size1];

    for (int i = 0; i < size1; i  ) {
        scanf("%d", &list1[i]);
    }

    int size2;
    printf("Enter list2: ");
    scanf("%d", &size2);

    int list2[size2];

    for (int i = 0; i < size2; i  ) {
        scanf("%d", &list2[i]);
    }

    int* list3 = merged(list1, list2, size1, size2);

    printf("The merged list is: ");

    int size3 = size1   size2;

    for (int i = 0; i < size3; i  ) {
        printf("%d ", list3[i]);
    }

    printf("\n");
}

int* merged(int list1[], int list2[], int size1, int size2) {
    int list3[size1   size2];
    for (int i = 0; i < size1; i  ) {
        list3[i] = list1[i];
    }

    int count = size1;

    for (int i = 0; i < size2; i  ) {
        list3[count] = list2[i];
        count  ;
    }

    int size3 = size1   size2;

    sort(list3, size3);

    int* list4;

    for (int i = 0; i < size3; i  ) {
        list4[i] = list3[i];
    }

    return list3;
}

void sort(int list3[], int size3) {

    for (int i = 0; i < size3; i  ) {
        int min = list3[i];
        int min_index = i;

        for (int j = i   1; j < size3; j  ) {
            if (list3[j] < min) {
                min = list3[j];
                min_index = j;
            }
        }

        if (min_index != i) {
            list3[min_index] = list3[i];
            list3[i] = min;
        }
    }
}

https://onlinegdb.com/-ChT7PA3w

CodePudding user response：

You merge return pointer to the local array. It is UB as this array stops existing when the function returns. So the returned pointer references an invalid object.

int *merged(const int * restrict list1, const int * restrict  list2, size_t size1, size_t size2) 
{
    int *result = NULL;
    int l1size = size1 * !!list1, l2size = size2 * !!list2;
    size_t newsize = l1size   l2size;

    if(newsize)
    {
        result = malloc(newsize * sizeof(*result));
        if(result)
        {
            if(l1size) memcpy(result, list1, l1size * sizeof(*result));
            if(l2size) memcpy(result   l1size, list2, l2size * sizeof(*result));
        }
    }
    return result;
}

void printList(const int * restrict list, size_t size)
{
    if(size && list)
    {
        for(size_t index = 0; index < size; index  )
            printf("[%2zu] = =\n", index, list[index]);
    }
}

void initList(int * restrict list, size_t size, int maxval)
{
    if(size && list)
    {
        for(size_t index = 0; index < size; index  )
            list[index] = rand() % maxval;
    }
}


int main() {

    size_t size1 = 20;
    size_t size2 = 10;
    int list1[size1], list2[size2];

    initList(list1, size1, 100);
    initList(list2, size2, 100);
    printList(list1, size1);
    printf("----------------\n");
    printList(list2, size2);
    printf("----------------\n");
    
    int *list3 = merged(list1, list2, size1, size2);
    printList(list3, size2   size1);
    printf("----------------\n");

    free(list3);
}

Also use the correct type for sizes size_t

CodePudding user response：

Couple of problems in your code:

First:
You are returning list3 from merged() function. list3 is a local(automatic) non-static variable and its lifetime is limited to its scope i.e. the block in which it has been declared. Any attempt to access it outside of its lifetime lead to undefined behaviour. Also, list3 is VLA (Variable Length Array), you cannot declare it static as VLA cannot have static storage duration. Other option is to declare list3 as pointer to int and allocate memory dynamically to it.

Second:
In the merged(), you are dereferencing uninitialised pointer list4. Dereferencing an uninitialised pointer is undefined behaviour. Allocate memory to list4 and then use it. Moreover, why you need list4? You copy the list1 and list2 to list3 and returning list3 from merged() function. I do not see any use of list4. Better to remove it from merged() function.

So, only one change is required in merged() in your code:

int*merged(int list1[], int list2[], int size1, int size2) {
    int * list3 = malloc ((size1   size2) * sizeof (int));
    if (list3 == NULL) {
        //handle the allocation failure
        //I am exiting..
        printf ("Failed to allocate memory\n");
        exit (EXIT_FAILURE);
    }
    ....
    ....
    ....
    sort (list3, size3);
    // removed list4 from code

    return list3;
}

Make sure to free the dynamically allocated memory. In the main() function, you should do:

int main (void) {
    ....
    ....
    ....
    for (int i = 0; i < size3; i  ) {
        printf("%d ", list3[i]);
    }

    printf("\n");
    // free dynamically allocated memory
    free (list3);

    return 0;
}

Couple of points, which I am leaving it up to you to learn and implement:

There is one serious problem in your code that you not validating the user input. Try giving value of size1 and size2 as 0 or negative value or very big positive value and check what happens. Handle the user input appropriately. Also, think over, if you really need VLA (Variable Length Array) or you can use fixed size array. A fixed size array can be static.
There is scope of improvement in implementation part. Try to find out them and make improvements.