0

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

2
  • Have you tried inspecting it using something like gdb and find what line and element is giving you the seg fault? Also, indenting your code better would help its readability Commented Dec 26, 2021 at 0:41
  • 1
    You cannot return a pointer to an automatic variable (such as list3). You cannot use a pointer variable which has not been initialized to point to some object (list4). If you want a function to return an array, you basically have two options: dynamically allocate the array with malloc or create the array in the caller and supply it as an additional parameter. Commented Dec 26, 2021 at 0:59

3 Answers 3

2

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] = %3d\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

Sign up to request clarification or add additional context in comments.

Comments

0

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.

2 Comments

It may be better form to not have merged allocate the memory for the merged list. If we take this responsibility away from merged then the memory can be automatically allocated in main and a pointer to it passed to merged. Merged having a declaration more like: int * merged(int * dest, int * list1, int * list1, size_t size1, size_t size2)
@Chris In C, It is perfectly normal for a function to allocate memory and return the reference of that memory and that's what merged() function is doing - returning a new list which is merge+sort of 2 list passed as argument to it. Beside this, read the last point of my post, I have clearly mentioned that There is scope of improvement in the implementation part and I left it on OP to identify it and make the respective changes in his code.
0

You cannot return list3 in merged, as you are returning the address of the first element (not the array), but the whole list3 array is out of scope and lifetime once the function returns, so it is no longer available (and the pointer is pointing to a place that is no longer valid for that purpose) You need to allocate memory dynamically to allow the array to survive the function call, or use a global variable (which makes the function non-reentrant)

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.