I'm new to programming and started learning more about merging two sorted arrays, but when I try to implement the code, I keep getting segmentation fault. I'm not really sure where my code went wrong and I was wondering if someone could look through it?

#include <stdlib.h>

struct Array {
    int A[10];  
    int size; 
    int length; 
}; 

struct Array* merge(struct Array *arr1, struct Array *arr2) {
    int i, j, k; 
    i=j=k=0; 

    struct Array *arr3 = (struct Array*)malloc(sizeof(struct Array)); 
    
    while (i < arr1->length && j < arr2->length) {
        if (arr1->A[i] > arr2->A[j]) {
            arr3->A[k] = arr2->A[j];
            k  , j  ; 
        } 

        if (arr1->A[i] < arr2->A[j]) {
            arr3->A[k] = arr1->A[i]; 
            k  , i  ; 
        }
    }

    for (; i < arr1->length; i  ) {
        arr3->A[k  ] = arr1->A[i]; 
    }

    for (; j < arr2->length; j  ) {
        arr3->A[k  ] = arr2->A[j];
    } 
    arr3->size = 10; 
    arr3->length = arr1->length   arr2->length; 

    return arr3; 
}

void Display(struct Array arr) {
    int i; 
    
    for (i = 0; i < arr.length; i  ) {
        printf("%d ", arr.A[i]); 
    }
}

int main() { 
    struct Array arr1 = {{2, 9, 21, 28, 35}, 10, 5}; 
    struct Array arr2 = {{2,3,16,18,28}, 10, 5};

    struct Array *arr3; 
    arr3 = merge(&arr1, &arr2);
    Display(*arr3); 

    return 0; 
}

CodePudding user response：

I don't get a segmentation fault, and I don't see why you would except from malloc failing from running out of memory. In that situation, a seg fault isn't really a bad thing, but a proper error message would be better.

However, it does produce an infinite loop because you don't handle the case where arr1->A[i] == arr2->A[j]. This can be fixed by replacing if (arr1->A[i] < arr2->A[j]) with else.

You also have a memory leak because of a missing free(arr3).

Note that int A[10] should be int *A so you can accommodate arrays of any size.

Note that I would write Display to take a pointer, allowing you to write Display(arr3) instead of Display(*arr3). That would be a tiny bit faster, but the real reason is because you'd normally already have a pointer, so it's simpler/cleaner.

CodePudding user response：

1. You do not need size in your code as your array has fixed length
2. Use flexible array members
3. Use the correct type for sizes (size_t)
4. Do not cast result of malloc. If your code is not compiling it means: you use C compiler to compile C code.
5. Your sorted arrays can have elements in a different order. You need to let know your merge function what elements order arrays use.

typedef struct 
{
    size_t size;
    int data[];
}Array_t; 

Array_t *merge(const Array_t *a1, const Array_t *a2, const int order)
{
    size_t a1index = 0, a2index = 0;
    Array_t *result = (a1 && a2) ? malloc(sizeof(*result)   (a2 -> size   a1 -> size) * sizeof(result -> data[0])) : NULL;

    if(result)
    {
        result -> size = a1 -> size   a2 -> size;
        for(size_t index = 0; index < result -> size; index  )
        {
            if(a1index >= a1 -> size) result -> data[index] = a2 -> data[a2index  ];
            else if(a2index >= a2 -> size) result -> data[index] = a1 -> data[a1index  ];
            else
            {
                int comp = order ? a1 -> data[a1index] > a2 -> data[a2index] : a1 -> data[a1index] < a2 -> data[a2index];
                if(comp) result -> data[index] = a1 -> data[a1index  ];
                else result -> data[index] = a2 -> data[a2index  ];
            }
        }
    }
    return result;
}

#define ASIZE 4
#define BSIZE 6

int main(void)
{
    //check for errors in real code -  I am too lazy
    Array_t *a = malloc(sizeof(*a)   ASIZE * sizeof(a -> data[0]));
    a -> size = ASIZE;
    Array_t *b = malloc(sizeof(*b)   BSIZE * sizeof(b -> data[0]));
    b -> size = BSIZE;

    Array_t *c;

    for(size_t i = 0; i < a -> size; i   ) a -> data[i] = i;
    for(size_t i = 0; i < b -> size; i   ) b -> data[i] = i;

    c = merge(a,b,0);
    for(size_t i = 0; i < c -> size; i   ) printf("[%zu] %d\n", i, c -> data[i]);

    free(c);

    for(size_t i = 0; i < a -> size; i   ) a -> data[i] = ASIZE - i;
    for(size_t i = 0; i < b -> size; i   ) b -> data[i] = BSIZE - i;

    c = merge(a,b,1);
    for(size_t i = 0; i < c -> size; i   ) printf("[%zu] %d\n", i, c -> data[i]);
    
    free(a);
    free(b);
    free(c);   
}

https://godbolt.org/z/5EcPEcjh1