Image writing function not executing

Question

I have a function that isn't executing when being called in my driver. I debugged it line-by-line by printf statements, yet I can't get any of them to appear no matter where I place them in my function. The function isn't producing the intended result -- to write to an image file.

void WriteImage(char *filename, Image &img)
{
    printf("writing\n");
    FILE *f = fopen(filename, "wb");

    if (f == NULL)
    {
        fprintf(stderr, "Can't open file %s to write.\n", filename);
        return;
    }

    fprintf(f, "P6\n");
    fprintf(f, "%d %d\n", img.GetX(), img.GetY());
    fprintf(f, "%d\n", 255);
    fwrite(img.GetData(), sizeof(PixelStruct), img.GetX() * img.GetY(), f);
    fclose(f);
}

This is code that was taken from a valid C function that I know works, so I don't know what's happening. Any ideas?

EDIT:

For what it's worth, it does execute when it is called by itself and only itself. When I call the preceding ReadImage beforehand, WriteImage does not execute. Here's ReadImage:

void ReadImage(char *filename, Image &output)
{
    FILE *f = fopen(filename, "rb");
    char magicNum[128];
    int  width, height, maxval;

    if (f == 0)
    {
        fprintf(stderr, "Unable to open file %s\n", filename);
        exit(0);
    }

    fscanf(f, "%s\n%d %d\n%d\n", magicNum, &width, &height, &maxval);

    printf("Magic num = %s width = %d, height = %d, maxval = %d\n",
            magicNum, width, height, maxval); 


    if (strcmp(magicNum, "P6") != 0)
    {
        fprintf(stderr, "Unable to read from file %s, because it is not a PNM file of type P6\n", filename);
        exit(1);
    }

    output.ResetSize(width, height);
    PixelStruct *data = output.GetData();
    output = Image(width, height, data);
    fread(output.GetData(), sizeof(PixelStruct), width*height, f);
    fclose(f);

}

EDIT 2: Main function.

int main(int argc, char *argv[])
{
    Image img;
    ReadImage(argv[1], img);
    printf("ReadImage called.\n");
    WriteImage(argv[2], img);
}

EDIT 3: The parametrized constructor.

Image::Image(int width, int height, PixelStruct* data)
{
    this->x = width;
    this->y = height;
    this->data = data;
}

EDIT 4: Image.

struct PixelStruct
{
    unsigned char red;
    unsigned char green;
    unsigned char blue;
};

class Image
{
        private:
            int x;
            int y;
            PixelStruct *data;

        public:
            Image(void); /* Default constructor */
            Image(int width, int height, PixelStruct* data); /* Parameterized constructor */
            Image(Image &); /* Copy constructor */
            void ResetSize(int width, int height);
            int GetX();
            int GetY();
            PixelStruct* GetData();
            void SetData(PixelStruct *data);
};

Image::Image(void)
{
    x = 0;
    y = 0;
    data = 0;
}
Image::Image(int width, int height, PixelStruct* data)
{
    this->x = width;
    this->y = height;
    this->data = data;
}

Image::Image(Image &img)
{
    this->x = img.x;
    this->y = img.y;
    this->data = img.data;
}

void Image::ResetSize(int width, int height)
{
    this->x = width;
    this->y = height;
}

int Image::GetX()
{
    return x;
}

int Image::GetY()
{
    return y;
}

PixelStruct* Image::GetData()
{
    return data;
}

Answer 1

This is only a partial answer, as there are much better ways to design your class, (also depending on whether you can use c++11 constructs or not) but the shortest way I can think of to fix your programm would be the following:

1.Allocate memory in your constructor and free it in your destructor:

Image::Image(int width, int height)
{
    this->x = width;
    this->y = height;
    this->data = new PixelStruct[width*height];
}

Image::~Image()
{
    delete[] data;
}

2.Delete the default constructer, copy constructor and resize method

3.Modify your Read Image function

Image* ReadImage(char *filename)
{
    .
            .
            .
            //read image size


    Image *output = new Image(width, height);
    fread(output->GetData(), sizeof(PixelStruct), width*height, f);
    fclose(f);

    return output;

}

4.modify your main to:

int main(int argc, char *argv[])
{

    Image* img= ReadImage(argv[1]);
    printf("ReadImage called.\n");
    WriteImage(argv[2], *img);
}

Again, this is NOT the recommended way to do it, but it should make your code work without too many modifications and Introduction of new concepts.

Answer 2

Your class has a number of memory management problems in regard the data member. Let me start with your specific problem

Image img;
ReadImage(argv[1], img);

At this point img has 0 for its buffer.

output.ResetSize(width, height);      // Buffer is still 0
PixelStruct *data = output.GetData(); // data is 0
output = Image(width, height, data);  // data is still zero

Here you have a memory problem. You don't have an assignment operator. In this particular case it has no real effect but is likely to kill you in the long run. You probably should just have a working

 ouput.ResetSize (width, height) ;

(as described below) without the other statements.

fread(output.GetData(), sizeof(PixelStruct), width*height, f); // data is still 0

I am surprised you don't get a blow out on the freed.

To fix:

These functions should probably allocate a buffer for data.

        Image(int width, int height, PixelStruct* data); 
        void ResetSize(int width, int height);

This function

        Image(Image &); /* Copy constructor */

should make a copy of data (not just assign the pointer). You need an assignment operator that does the same but should delete any existing data.

You should have a destructor that frees data.