I'm not sure which is your line #59 (I only get #55 pasting your code in my editor ;).
I found some details:
On line #17 (my line #17) you wrote:
if(file == NULL)`
There's an extra '`' after the brackets. (probably it's not there in your code: it would not even compile it)
On line #25:
while ((fread(&buffer, sizeof(BLOCK),1,file)) == 1)
I'm pretty sure that's what's causing the 'segfault', though indirectly (we'll come back to that later:). Besides, I'm pretty sure it's not what you're trying to do. 'BLOCK' is a constant (an 'int', actually) and as such, is the size of an int ( = 4, on my system). So you're actually telling your system to read in 1 item at a time, of size 4 bytes, and store it into 'buffer'.
What you were (probably) trying to do is:
fread(&buffer, sizeof(buffer),1,file)
(try printing out the size of all those variables/constants)
Lines #28 and #29:
//read 512 bytes into a buffer
fread(&buffer, sizeof(BLOCK),1,file);
Inside the loop, you read in some bytes again. 'C' will actually read in twice every time through your loop. So, you'll be overwriting 'buffer' with new information, and loosing the bytes already read in when checking the while condition.
So, here's the deal:
When you enter the 'while' loop for the first time, 'C'checks the condition:
fread(&buffer, sizeof(BLOCK),1,file)) == 1
According to it's man page:
On success, fread() and fwrite() return the number of items read or
written. This number equals the number of bytes transferred only when
size is 1.
So, since you're reading in 4 bytes (as already stated), the function returns = 4. That means 'C' compares:
4 == 1 --> false
It will never enter the loop.
Therefore, when it gets to line #53:
fclose(img);
it will try to close a file that's never been opened. According to it's man page:
The behaviour of fclose() is undefined if the stream parameter is an
illegal pointer, or is a descriptor already passed to a previous invocation of fclose().
I'm not totally sure, but pretty confident that passing in a 'NULL' pointer is really close to passing in "an illegal pointer".
HTH!
P.S.: you should always try inserting 'printf()' statements inside your code, bisecting it, and try to reach a conclusion as to which is the line producing the error. In this case, you would've been able to see that you are never entering the while loop, and that could've been a guide as on where to start the bug hunting ;)
EDIT:
I made a few modifications on your new version (v2) of your code:
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#define BLOCK 512
typedef uint8_t BYTE;
int main(void)
{
printf("%s\n", "Test1");
BYTE jpeg[512];
BYTE buffer[BLOCK];
char title[8];
int pcount = 0;
FILE* img = NULL;
int read; /* edit */
printf("%s\n", "Test2");
//open card file
FILE* file = fopen("card.raw","r");
printf("%s\n", "Test3");
if(file == NULL)
{
printf("Could not open memory card.\n");
}
printf("%s\n", "Test4");
printf("%ld\n",sizeof(buffer));
printf("%s\n", "Test5");
printf("%ld\n",sizeof(BLOCK));
printf("%s\n", "Test6");
read = fread(&buffer, sizeof(buffer),1,file);
printf("%ld\n", read);
printf("%s\n", "Test7");
//repeat until end of card
while ((fread(&buffer, sizeof(buffer),1,file)) == 1)
{
printf("%s\n", "INSIDE WHILE: Test8");
//if start of a new JPG
if (jpeg[0] == 0xff && jpeg[1] == 0xd8 && jpeg[2] == 0xff && (jpeg[3] == 0xe0 || jpeg[3] == 0xe1) )
{
//if image has not been opened
if(img == NULL)
{
sprintf(title,"%3d.jpg",pcount);
img = fopen(title,"a");
fwrite(&buffer, sizeof(buffer), 1, img);
pcount++;
}
//if image has already been opened
else if(img!= NULL)
{
fclose(img);
}
}
else
{
fwrite(&buffer, sizeof(buffer), 1, img);
}
}
fclose(img);
fclose(file);
}
Try it yourself, and see if you can spot something. First thing you'll notice is that printf statements do print out prior to the segfault.
A couple more hints I'll give you: when you fread() or fwrite() from/to a *FILE stream, 'C' is responsible for "moving the cursor" around it. All you need to do is "call" it repeteadly, and the function will return sequential chunks of data.
There is, though, one precaution you should take: when you fopen() a file, the function loads the data into a memory address you passed in as an argument (that would be buffer). When you repeteadly keep calling fopen() (in the while loop) the cursor keeps moving forward in memory.
But, according to fread()'s man page:
If an error occurs, or the end of the file is reached, the return value is a short item count (or zero). fread() does not distinguish between end-of-file and error, and callers must use feof(3) and ferror(3) to determine which occurred.
So, you should either:
add a new feof(file) condition to your while loop
test the last fread() returned value, after leaving the while loop, to confirm whether you crushed into a read error, or you actually finished reading your file successfully.
This "auto-moving-the-cursor" has another consequence: if, like you did on your v2 code (I changed it) line #35, print out the result of fread():
printf("%d",fread(&buffer, sizeof(buffer),1,file));
prior to your actual read procedure (in your case, it would actually start with the while loop), the cursor will not be at the position you intend it to be (i.e.: at the very beginning of you file) when you actually want to start reading in the info.
There's more to review in your code, but I'm hoping I've convinced you to start printing out values (and even dummy lines) to be able to identify problems more accurately.
I was told by a teacher once:
Don't ever try to guess/find why your code don't do what it's supposed to do. That's, most of the time, a loss of time. Instead, always try to find out and understand why it does what it does. That's almost always a better starting point for a bug-hunting process.
Try it out and see if you get any closer! :)
(of course, come back and I'll be happy to help again ;)
