-3

I'm quite new to programming, and I'm trying to write a basic program to copy a jpeg file:

#include <stdio.h>

int main (void)
{
    FILE *a = fopen("pic1.jpg", "r");
    
    FILE *b = fopen("pic2.jpg", "w");
    
    fread(a, 1, sizeof(a), b);
    fwrite(a, 1, sizeof(a), b);
    
    fclose(a);
    fclose(b);
}

The program does create a new file pic2.jpg, however, when I try to look at it, I receive the error message: Invalid or Unsupported Image Format, so this makes me think I may be trying to use this function incorrectly.

BlueKhakis
  • 293
  • 1
  • 8
  • 3
    use open modes `"rb"` and `"wb"` respectively. Check documentation of `fopen` to see why. Also the fread and fwrite calls are both incorrect , you need to supply a memory region to read into (there is no standard function to read one file and write the other file in one go) – M.M Feb 14 '21 at 01:00
  • 5
    `sizeof(a)` doesn't mean what you think it means. – hobbs Feb 14 '21 at 01:02
  • Hi @hobbs can you expand on that? – BlueKhakis Feb 14 '21 at 01:07
  • 1
    `sizeof(a)` gets the size of the pointer; to get the *file size*, see https://stackoverflow.com/questions/238603/how-can-i-get-a-files-size-in-c – SuperStormer Feb 14 '21 at 01:08
  • Also, `fread()` reads into a buffer that you need to supply, and `fwrite()` writes out the contents of an analogous buffer. You are attempting to use the `FILE` object to which `a` points in that role, but it is completely unsuitable for the purpose. – John Bollinger Feb 14 '21 at 01:29
  • https://stackoverflow.com/a/5263102/920069 – Retired Ninja Feb 14 '21 at 01:38
  • 3
    The size of the external file is probably a red herring. The usual approach would be to declare a `char` array into which to read a small number of blocks at a time -- maybe 4 KiB or so. Then you loop, filling the buffer from the input file with `fread()` and writing the results to the output file with `fwrite()`, until you get a short (or zero-byte) read, which signals end-of-file. After you write the last, partial block, you close the files and are done. – John Bollinger Feb 14 '21 at 01:39

1 Answers1

3

Yes, but you're not using them correctly.

fread(a, 1, sizeof(a), b) says to read 1 byte from b sizeof(a) times into the FILE *a. This compiles, but doesn't make much sense. You make a similar mistake with fwrite.

sizeof(a) gets the size of a FILE * pointer which is usually 8 bytes, not the size of the file. So you're only reading and writing 8 bytes.

The first argument of fread and fwrite is a buffer to read into and write from, that's the key element which is missing.


First, always check your file operations worked. If they fail, if the file doesn't exist or can't be opened, the program will continue merrily along and you'll get weird errors when it tries to write to NULL.

Some systems make a distinction between "binary" and "text" files. You have to add the b flag to make sure they don't mangle the content.

I'll also use more descriptive names so we don't get confused.

FILE *in = fopen("pic1.jpg", "rb");
if( !in ) {
    perror("can't open pic1.jpg for reading");
}

FILE *out = fopen("pic2.jpg", "wb");
if( !out ) {
    perror("can't open pic2.jpg for writing");
}

Now we need to allocate a buffer to read into and write from. For best performance this should be something like the natural block size of the disk. 4096 or the BUFSIZ constant are good choices. We use char as a surrogate for an array of bytes. If you want to get fancy you could use a uint8_t but since we're just copying bytes there's no practical difference.

Read into that and write from that in a loop until everything is read.

char buf[BUFSIZ];
size_t bytes_read;

// Read a buffer sized hunk.
while( (bytes_read = fread(buf, 1, sizeof(buf), in)) ) {
    // Write the hunk, but only as much as was read.
    fwrite(buf, 1, bytes_read, out);
}

fclose(in);
fclose(out);

We can use sizeof(buf) here only because buf is an array of a known size. It will return the size of the buffer, not the size of a char * pointer.

We have to be careful to write only as much as we read, not the whole buffer. Otherwise if we did fwrite(buf, 1, sizeof(buf), out) and the last read only partially filled the buffer we'd print garbage.

fread returns the number of "items" read, not the number of bytes. This is from waaay back when files were more likely to have fixed size records. To get it to return the number of bytes we tell it each "item" is 1 byte and we want sizeof(buf) "items".


Schwern
  • 153,029
  • 25
  • 195
  • 336