fread seems to break my program

Question

I've been spending a lot of time trying to get my head around the fread and fwrite functions. In said (vain) attempts, I made some little changes to the copy.c program that is provided as part of pset4 and seem to be having some bizarre problems.

The original code iterates over the infile's scanlines and copies each individual pixel one at a time. I thought it would be possible to change this so it reads the full scanline and copies it to the outfile once only (rather than going pixel by pixel). I tried to implement it as follows:

// Copies a BMP file

#include <stdio.h>
#include <stdlib.h>

#include "bmp.h"

int main(int argc, char *argv[])
{
    // ensure proper usage
    if (argc != 3)
    {
        fprintf(stderr, "Usage: copy infile outfile\n");
        return 1;
    }

    // remember filenames
    char *infile = argv[1];
    char *outfile = argv[2];

    // open input file
    FILE *inptr = fopen(infile, "r");
    if (inptr == NULL)
    {
        fprintf(stderr, "Could not open %s.\n", infile);
        return 2;
    }

    // open output file
    FILE *outptr = fopen(outfile, "w");
    if (outptr == NULL)
    {
        fclose(inptr);
        fprintf(stderr, "Could not create %s.\n", outfile);
        return 3;
    }

    // read infile's BITMAPFILEHEADER
    BITMAPFILEHEADER bf;
    fread(&bf, sizeof(BITMAPFILEHEADER), 1, inptr);

    // read infile's BITMAPINFOHEADER
    BITMAPINFOHEADER bi;
    fread(&bi, sizeof(BITMAPINFOHEADER), 1, inptr);

    // ensure infile is (likely) a 24-bit uncompressed BMP 4.0
    if (bf.bfType != 0x4d42 || bf.bfOffBits != 54 || bi.biSize != 40 ||
        bi.biBitCount != 24 || bi.biCompression != 0)
    {
        fclose(outptr);
        fclose(inptr);
        fprintf(stderr, "Unsupported file format.\n");
        return 4;
    }

    // write outfile's BITMAPFILEHEADER
    fwrite(&bf, sizeof(BITMAPFILEHEADER), 1, outptr);


    // write outfile's BITMAPINFOHEADER
    fwrite(&bi, sizeof(BITMAPINFOHEADER), 1, outptr);

    // determine padding for scanlines
    int padding = (4 - (bi.biWidth * sizeof(RGBTRIPLE)) % 4) % 4;

    // iterate over infile's scanlines
    for (int i = 0, biHeight = abs(bi.biHeight); i < biHeight; i++)
    {
        // temporary storage for full scanline
        RGBTRIPLE *scanline = calloc(bi.biWidth, sizeof(RGBTRIPLE));

        // read scanline from infile
        fread(&scanline, sizeof(RGBTRIPLE), bi.biWidth, inptr);

        // write write full scanline to outfil
        fwrite(&scanline, sizeof(RGBTRIPLE), bi.biWidth, outptr);


        // skip over padding, if any
        fseek(inptr, padding, SEEK_CUR);

        // then add it back (to demonstrate how)
        for (int k = 0; k < padding; k++)
        {
            fputc(0x00, outptr);
        }
    }

    // close infile
    fclose(inptr);

    // close outfile
    fclose(outptr);

    // success
    return 0;
}

This compiles fine, but then ends up getting stuck in a loop when it's run to the point that you eventually get a "File size limit exceeded" message. I've run debug50 and the problem seems to be that once the code enters the for loop and gets to the line:

fread(&scanline, sizeof(RGBTRIPLE), bi.biWidth, inptr);

the values for both i and k suddenly change to massive numbers inexplicably! So the program never get's past the for loop at the bottom without running out of memory because it's adding near infinite amounts of padding to the outfile! I've tried messing around with the temporary memory allocation and with the the fread and fwrite funcitons themselves, but don't seem to have any luck at all other than using the exact examples given in the copy example (i.e. just going over each individual pixel one at a time).

I've spent a stupid amount of time reading about fread and fwrite and can't find any explanation as to why this is happening! Any help would be greatly appreaciated :)

Answer 1

Use scanline, not &scanline. It already is a pointer to the allocated area. &scanline is a pointer to a pointer.

The weird part of this: If scanline were an array, both would work the same. I prefer the scanline version even in that case, because arrays in C are like pointers in so many ways.

And your memory management is poor.

Either use something like

RGBTRIPLE scanline[bi.biWidth];

That variable is stored on the stack, and the compiler will forget about the variable once you leave the scope it was declared in, when it resets the stack pointer to its old value (the space will be used by the next variable declared, which for variables declared in a loop will likely be the same variable in the next iteration).

Or have a free for each malloc/calloc. If you calloc within the loop, you need to free before you leave the block (otherwise your code would lose track of the memory). If you calloc before the loop, and reuse the memory all the time, you need only one free after the loop.