Problem with freeing memory in c++ code due to invalid pointer

Question

I have simple code that calculates PSNR. I have two type of images, with signed and unsigned data, so I have to cover both cases in my code. Now, when I run code, I get printed right PSNR value, however I get the following error:

munmap_chunk(): invalid pointer

Aborted (core dumped)

I know it has something with pointers. I would appreciate if someone can point me to the problem, so I can explore further in more details. And also, what would be the best solution in this particular case? The code is below.

#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <math.h>
#include "string.h"
#include <fstream>


int main(int argc, char *argv[])
{
    int i, w, h, t, b; 
    double mse, d, psnr;
    bool isSigned;
    FILE    *in_file1, *in_file2;

    int *image5;
    int *image6;
    unsigned int *image7;
    unsigned int *image8;

    if(argc!=7){
        printf("Usage: %s file1 file2 width height bit_resolution is_signed\n",argv[0]);
        exit(1);
    }

    in_file1=fopen(argv[1],"rb");
    in_file2=fopen(argv[2],"rb");
    w=atoi(argv[3]);
    h=atoi(argv[4]);
    b=atoi(argv[5]);
    isSigned = !strcmp(argv[6], "true");
    t=w*h;


    if (isSigned){
        image5=(int *)calloc(t, sizeof(int));
        image6=(int *)calloc(t, sizeof(int));

        fread(image5,sizeof(int),w*h,in_file1);
        fread(image6,sizeof(int),w*h,in_file2);
    }
    else{

        image7=(unsigned int *)calloc(t, sizeof(unsigned int));
        image8=(unsigned int *)calloc(t, sizeof(unsigned int));

        fread(image7,sizeof(unsigned int),w*h,in_file1);
        fread(image8,sizeof(unsigned int),w*h,in_file2);
    }


    mse=0.;
    if(isSigned)
    {
        for(i=0;i<t;i++){
            d=(double)image5[i]-(double)image6[i]; 
            mse+=d*d;
        }
    }
    else
    {
        for(i=0;i<t;i++){
            d=(double)image7[i]-(double)image8[i]; 
            mse+=d*d;
        }
    }
    mse=mse/t;

    d=1.;
    for(i=0;i<b;i++) d*=2;
    d-=1.;

    psnr=20*log10(d)-10*log10(mse);

    printf("P=%f MSE=%f PSNR=%fdB\n",d, mse, psnr);

    free(image5);
    free(image6);
    free(image7);
    free(image8);

    return 0;
}

Answer 1

you free all inconditionaly :

free(image5);
free(image6);
free(image7);
free(image8);

but

if (isSigned){
    image5=(int *)calloc(t, sizeof(int));
    image6=(int *)calloc(t, sizeof(int));
    ...
}
else{
    image7=(unsigned int *)calloc(t, sizeof(unsigned int));
    image8=(unsigned int *)calloc(t, sizeof(unsigned int));
     ...
}

if isSigned then image7 and image8 are not initialized, else image5 and image6 are not initialized, the behavior is undefined

initialize all theses pointer to NULL or if isSigned just free image5 and image6 else only image7 and image8

---

Are you sure the tag C++ must not be C ?

---

Doing

w=atoi(argv[3]);
h=atoi(argv[4]);
b=atoi(argv[5]);

is unsure, if the argument are not valid number atoi silently returns 0, I recommand you to never use atoi but strtol or scanf checking their result

You also access to argv without checking first argc, if not enough arguments are given the behavior is undefined

you also do not check you was able to open the files, and you do not check fread result