2

I'm making a library for reading PGM files and I got stuck with this problem.

My code can't read binary PGM images correctly, it looks like it reads the wrong values, thus generating an image with only "noise"

The code is really simple:

void OpenPGM(PGMImage* pgm, const char* file){
    FILE *pgmfile = fopen (file, "rb");

    fscanf (pgmfile, "%s", pgm->magicNumber);
    fscanf (pgmfile, "%d %d", &(pgm->width),&(pgm->height));
    fscanf (pgmfile, "%d", &(pgm->maxValue));

    pgm->data = malloc(pgm->height * sizeof(unsigned char*));

    if (pgm->magicNumber[1] == '2')
    {
        for (int i = 0; i < pgm->height; ++i)
        {
            pgm->data[i] = (unsigned char*)malloc(pgm->width * sizeof(unsigned char*));
            for (int j = 0; j < pgm->width; ++j)            
                fscanf (pgmfile, "%d", &pgm->data[i][j]);           
        }
    } else {
        fgetc(pgmfile);// this should eat the last \n
        for (int i = 0; i < pgm->height; ++i)
        {
            pgm->data[i] = (unsigned char*)malloc(pgm->width * sizeof(unsigned char*));
            fread(pgm->data[i],sizeof(unsigned char*),pgm->width,pgmfile);//reading line by line
        }
    }
}

and the PGMImage looks like this

typedef struct PGMImage {
    char magicNumber[2];
    unsigned char** data;
    unsigned int width;
    unsigned int height;
    unsigned int maxValue;
} PGMImage;

What am I doing wrong?

1 Answers1

1

There is a potential issue as you read the image:

pgm->data[i] = (unsigned char*)malloc(pgm->width * sizeof(unsigned char*));
fread(pgm->data[i],sizeof(unsigned char*),pgm->width,pgmfile);//reading line by line

should be:

pgm->data[i] = malloc(pgm->width * sizeof(unsigned char));
if(pgm->data[i]==NULL){fprintf(stderr,"malloc failed\n");exit(1);}
fread(pgm->data[i],sizeof(unsigned char),pgm->width,pgmfile);//reading line by line

Indeed, unsigned char* is a pointer to an unsigned char and sizeof(unsigned char*) will be the size of a pointer (likely 8 bytes). Hence, the the image is read, 8 rows get read each time you read a line.

francis
  • 9,525
  • 2
  • 25
  • 41
  • Wow, you are absolutely right. Damn, pointers are tricky. But even with this fix it still doesn't work, the image is still noisy. I'll edit my post with some more info. – Luiz Eduardo Simões Sep 24 '16 at 19:42
  • Ok, now the problem was easy to find. I was printing the original magic numbers on the picture but the info after reading were always printed in ASCII format, so it always should be P2. Thank you for the help! – Luiz Eduardo Simões Sep 24 '16 at 19:51