Thread: Reading a PCX file into memory

Hi,

I've been trying to create a PCX "object" with some handy members, using the following code (includes and unnecessary functions cut):-

Code:

typedef struct {
   unsigned char manufacturer; /* = 10 */
   unsigned char version;
   unsigned char encoding;
   unsigned char bitsperpixel;
   int x;
   int y;
   int width;
   int height;
} t_pcx_header;

typedef struct {
   int width;
   int height;
   unsigned char *picdata;
} t_pic;

t_pic *load_pcx(char *filename)
{
   FILE *fp;
   t_pcx_header pcx;
   t_pic *pic;
   int databyte;
   int count;
   long l;

   fp = fopen(filename, "rb");
   if (fp == NULL)
   {
      printf("File error\n");
      return NULL;
   }
   fread(&pcx, sizeof(pcx), 1, fp);
   if (pcx.manufacturer != 10)
   {
      printf("Invalid manufacturer value %d\n", pcx.manufacturer);
      return NULL;
   }
   else if (pcx.encoding != 1)
   {
      printf("Invalid encoding value %d\n", pcx.encoding);
      return NULL;
   }
   else if (pcx.bitsperpixel != 8)
   {
      printf("Invalid bits per pixel value %d\n", pcx.bitsperpixel);
      return NULL;
   }
   else if (pcx.x < 0 || pcx.x > 319 || pcx.y < 0 || pcx.y > 199 || pcx.width < 1 || pcx.width > 319 || pcx.height < 1 || pcx.height > 199)
   {
      printf("Invalid image dimensions x=%d, y=%d, width=%d, height=%d\n", pcx.x, pcx.y, pcx.width, pcx.height);
      return NULL;
   }
   fseek(fp, 128, SEEK_SET);
   pic = (t_pic *)malloc(sizeof(t_pic));
   if (pic->picdata == NULL)
   {
      printf("Picture object: Memory allocation error\n");
      return NULL;
   }
   pic->width = pcx.width + 1;
   pic->height = pcx.height + 1;
   pic->picdata = (unsigned char *)malloc((long)pic->width * pic->height);
   if (pic->picdata == NULL)
   {
      printf("Picture data: Memory allocation error.\n");
      return NULL;
   }
   for (l=0;l<(long)pic->width * pic->height;l++)
   {
      databyte = getc(fp);
      if (databyte == EOF)
      {
	 printf("Image read error\n");
	 return NULL;
      }
      if (databyte & 0xC0)
      {
	 unsigned char j;

	 count = databyte & 0x3F;
	 databyte = getc(fp);
	 printf("%d\n", count);
	 for (j=0;j<count;j++)
	    pic->picdata[l++] = (unsigned char)databyte;
      }
      else
	 pic->picdata[l] = (unsigned char)databyte;
   }
   fclose(fp);
   return pic;
}

I've tried compiling and running this using Borland Turbo C 2.0, but it keeps crashing (none of my amazing error checking being done ), and I imagine it has something to do with the way I handle pointers. Any ideas?

Apart from this
pic = (t_pic *)malloc(sizeof(t_pic));
if (pic->picdata == NULL)
{
printf("Picture object: Memory allocation error\n");
return NULL;
}

Should be
pic = (t_pic *)malloc(sizeof(t_pic));
if (pic == NULL)
{
printf("Picture object: Memory allocation error\n");
return NULL;
}

> pic->picdata = (unsigned char *)malloc((long)pic->width * pic->height)
If you're using a 16 bit compiler, you'd better watch this - if length*height is >64K, it will truncate it without warning, and you'll have a lot less memory than you thought you had.

> fwrite(data->picdata, data->width * data->height, 1, fp);
Gotta watch those multipliers - it only takes 256*256 to overflow a short integer

Add some tracing statements to print out
  printf( "size=%ld\n", (long)width * (long) height );
wherever you do this multiplication. If its larger than 32768, then you might have a problem, and if its larger than 65536, then you almost certainly have a problem.