0

I am working on pset4 filter -r (reflection).

How I want to achieve reflection:

  1. Reserve space in memory, enough to fit an image. Name it image-copy.
  2. Copy 1-to-1 the original file there. Original image is in image-copy now.
  3. Re-write each row in a way that 1st pixel from image-copy goes to the last pixel position in image.
  4. Free memory reserved for image-copy.

This way of doing makes wanted (reflected) image to be stored in image, without need of copying/manipulating memory after reflection.

It works fine, anyway after help50 valgrind ./filter -r images/yard.bmp yardout.bmp the output is ERROR SUMMARY: 3 errors from 2 contexts (suppressed: 0 from 0).
Analysing what was output from valgrind I found:

==1614== Invalid write of size 2
==1614==    at 0x426D3E: reflect (helpers.c:87)
==1614==    by 0x42332B: main (filter.c:115)
==1614==  Address 0x5ebf240 is 0 bytes after a block of size 720,000 alloc'd
==1614==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1614==    by 0x423046: main (filter.c:78)
==1614== 
==1614== Invalid write of size 1
==1614==    at 0x426D45: reflect (helpers.c:87)
==1614==    by 0x42332B: main (filter.c:115)
==1614==  Address 0x5ebf242 is 2 bytes after a block of size 720,000 alloc'd
==1614==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==1614==    by 0x423046: main (filter.c:78)

And helpers.c part of file looks like this (line 87 marked in comment):

// Reflect image horizontally
void reflect(int height, int width, RGBTRIPLE image[height][width])
{
    // re-write from left to right

    // reserve space for a copy:
    RGBTRIPLE(*image_copy)[width] = calloc(height, width * sizeof(RGBTRIPLE));
    if (image_copy == NULL)
    {
        printf("Not enough memory to create copy of an image\n");
        return;
    }
    // make a copy of image
    for(int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            image_copy[i][j] = image[i][j];
        }
    }
    //rewrite in opposite - each row
    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            image[i][width - j] = image_copy[i][j]; // <-- line 87
        }
    }
    //free the memory
    free(image_copy);
    return;
}

filter.c line 115 is where there is a call for reflect()
filter.c line 78 is where calloc is:

// Allocate memory for image
RGBTRIPLE(*image)[width] = calloc(height, width * sizeof(RGBTRIPLE));  

Is it a serious problem? The picture resulted from this is reflected version of original picture.

1

Off by one. If j is 0, you access image[i][width] (a valid index would be from 0 to width-1), which due to memory layout would refer to image[i+1][0]. For the last line, that's behind the allocated memory.

Fix this by using width - 1 - j instead of width - j.

2
  • Yes, it is working now. Thank you. But.... Why no segmentation fault? – smajli Feb 13 '20 at 16:10
  • 1
    Segmentation fault happens when you access a segment you are not meant to. Those are larger units. Your programme won't ask the operating system for RAM each time you malloc (would be much slower), it gets some in advance, so while your access got out of the malloced range, it was still within the same segment associated to your process. This kind of mistake might not even be reported if that range also contained malloced memory. – Blauelf Feb 13 '20 at 16:17

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .