CODEWITHSUNDEEP
cmultidimensional-arraysegmentation-faultmemcpy
Butanium
Butanium

Reputation: 790

memcpy error segmentation fault while trying to copy 2D array of 4 char structure in C

Hey I'm trying to copy an array of SDL_Color created from an image in another one. But for some images, I get :

Process finished with exit code -1073741819 (0xC0000005)

It happens for an image of 20 x 20 pixels but it works well for a 50 x 50 one... here is my code :

FILE *debugFile = fopen("C:\\Users\\Clement\\Documents\\coding\\ImageOfCLife\\debug.txt", "w+");
int imgWidth, imgHeight, channels;
unsigned char *img = stbi_load("C:\\Users\\Clement\\Documents\\coding\\ImageOfCLife\\star.jpg", &imgWidth,
                               &imgHeight, &channels, 0);
fprintf(debugFile, "Loaded image with a width of %dpx, a imgHeight of %dpx and %d channels\n", imgWidth, imgHeight, channels);
dRulesLen = sizeof(deathRules);
bRulesLen = sizeof(birthRules);
if (img == NULL) {
    fprintf(debugFile, "Error in loading the image\n");
    exit(3);
}

int ch, pix;
SDL_Color **stateMatrix1 = (SDL_Color **) malloc(imgHeight * sizeof(SDL_Color*));
if (stateMatrix1 == NULL) {
    fprintf(debugFile,"Unable to allocate memory\n");
    exit(1);
}
for (int i = 0; i < imgHeight; ++i) {
    stateMatrix1[i] = (SDL_Color *) malloc(imgWidth * sizeof(SDL_Color));
}
for (ch = 0; ch < imgHeight; ch++) {
    printf("{");
    for (pix = 0; pix < imgWidth; pix++) {
        unsigned bytePerSDL_Color = channels;
        unsigned char *SDL_ColorOffset = img + (pix + imgHeight * ch) * bytePerSDL_Color;
        SDL_Color p = initSDL_Color(SDL_ColorOffset);
        stateMatrix1[ch][pix] = p;
        printSDL_Color(p);
        printf(", ");
    }
    printf("}\n");
}
SDL_Color stateMatrix2[imgHeight][imgWidth];
memcpy(stateMatrix2, stateMatrix1, imgWidth*imgHeight*sizeof(SDL_Color));

the last line is the problem according to the debugger I tried

memcpy(stateMatrix2, stateMatrix1, sizeof(stateMatrix2))

too but I get the same result.

I work on windows 10 with minGW and Clion. I hope you can help me with this issue.

I also tried to replace SDL_Color stateMatrix2[imgHeight][imgWidth]; by :

    SDL_Color **stateMatrix2 = (SDL_Color **) malloc(imgHeight * sizeof(SDL_Color*));
if (stateMatrix2 == NULL) {
    fprintf(debugFile,"Unable to allocate memory\n");
    exit(1);
}
for (int i = 0; i < imgHeight; ++i) {
    stateMatrix2[i] = (SDL_Color *) malloc(imgWidth * sizeof(SDL_Color));
}

but i got the same issue.

I forgot to say it but I want ant to be able to use both stateMatrix as parameter of a function.

To fix it I used the Olaf solution explained below : I kept :

SDL_Color **stateMatrix1 = (SDL_Color **) malloc(imgHeight * sizeof(SDL_Color*));
if (stateMatrix1 == NULL) {
    fprintf(debugFile,"Unable to allocate memory\n");
    exit(1);
}
for (int i = 0; i < imgHeight; ++i) {
    stateMatrix1[i] = (SDL_Color *) malloc(imgWidth * sizeof(SDL_Color));
}

to allocate memory to both matrix and used :

for (int i = 0; i < imgHeight; ++i) {
memcpy(stateMatrix2[i], stateMatrix1[i], imgWidth * sizeof(SDL_Color));
}

to perform the copy. I also verified that the two matrix were'nt linked and there were no problem.

Upvotes: 0

Views: 230

Answers (2)

Olaf Dietsche
Olaf Dietsche

Reputation: 74078

When copying

memcpy(stateMatrix2, stateMatrix1, imgWidth * imgHeight * sizeof(SDL_Color));

you will go beyond the end of stateMatrix1, which is not imgWidth * imgHeight * sizeof(SDL_Color), but imgHeight * sizeof(SDL_Color*).

Copying in a loop from stateMatrix1 to stateMatrix2 is one way to solve this

for (int i = 0; i < imgHeight; ++i) {
    memcpy(stateMatrix2[i], stateMatrix1[i], imgWidth * sizeof(SDL_Color));
}

Another would be making both matrices the same type. But when allocating stateMatrix2 as

SDL_Color **stateMatrix2 = (SDL_Color **) malloc(imgHeight * sizeof(SDL_Color*));

and later use the same memcpy as above, you will still go beyond stateMatrix1 and now also beyond the end of stateMatrix2.

The correct way to copy (but still wrong for another reason) would be

memcpy(stateMatrix2, stateMatrix1, sizeof(*stateMatrix1));

This is correct in terms of size, but still wrong, because it copies the pointers of stateMatrix1 to stateMatrix2. This has two effects

  1. When you initialized stateMatrix2 with its own pointers, there will be memory leaks.
  2. Now both matrices point to the same memory, which means changing one, will change the other too.

Upvotes: 1

dspr
dspr

Reputation: 2433

Your error is explained in comments. You could fix it by allocating a contiguous block of memory for your matrix instead of many blocks as you did. This way of doing is even simpler, for allocation, deallocation and copy (since you can then use a single call to memcpy) :

#include <stdio.h>
#include <stdlib.h>

struct SDL_Color {
    unsigned char rgba[4];
};

int main() {
    int ch, pix;
    int imgWidth = 100;
    int imgHeight = 100;
    struct SDL_Color (*stateMatrix1)[imgWidth][imgHeight] = malloc(sizeof(*stateMatrix1));
    if (*stateMatrix1 == NULL) {
        printf("Unable to allocate memory\n");
    } else {
        free(*stateMatrix1);
        printf("Success\n");
    }
    return 0;
}

You just need to take care of dereferencing the pointer to the matrix each time you use it.

Upvotes: 0

Related Questions