0

My program is stuck in a loop and I can't figure out why. The problem is in the do while loop but logically it should work. The program checks performs fwrite and then freads a new 512 bytes to the buffer. Lastly, it checks whether the first 4 bytes of this new buffer is another JPEG. If it is not, then it continues writing. The problem is that it can't detect the next JPEG file!

while (!feof(card))
{
    // check for JPEG signature patterns      
    if (memcmp (&pattern1, &buffer, 4) == 0 || memcmp(&pattern2, &buffer, 4) == 0)
    {
        // create file name and increment by 1 for each JPEG
        sprintf(filename, "%03d.jpg", counter);
        counter++;

        // open a new file with the proper file name
        FILE* image = fopen(filename,"a");

        // check for null
        if(image == NULL)
        {
            fclose(card);
            fprintf(stderr, "Could not create %s.\n", filename);
        }

        // write to file and read the next 512 bytes
        do
        {
            fwrite(&buffer, sizeof(buffer), 1, image);
            fread(buffer, sizeof(BYTE), 512, card);
        }
        while(memcmp (&pattern1, &buffer, 4) != 0 || memcmp(&pattern2, &buffer, 4) != 0);
1
  • you should post your whole code. E.g.: we cannot see how you declared / initialized "BYTE" or "buffer" – abelinux Dec 3 '14 at 22:37
2

I think it's because you used an OR operator instead of an AND operator.

The jpg header will match either the first pattern or the second pattern, but not both. The only way your while loop will break now is if both patterns are matched, which is not possible. You'd want an && in the do while not a ||.

What you have now:

do
{
   //this
}
while(PATTERN A NOT MATCHED OR PATTERN B NOT MATCHED)

Let's use a simplified example, where the two patterns we are looking for are either 123 or 124 and matching either one of those would indicate the start of a file type that we are looking for.

    do
    {
       //this
    }
    while( Buffer is not 123   OR    Buffer is not 124)

So this seems good in theory, but when you break it down, the only way the while loop is going to break would be if BOTH patterns were matched at the same time. That's not technically possible since your buffer can't be equal to both 123 and 124 at the same time. So if your buffer held a value of 123, it would say, "well, that matches part A BUT Part B still doesn't match so keep running the while loop."

What the and does is

    do
    {
       //this
    }
    while( buffer is not 123  AND  buffer is not 124)

Which effectively says, as long as BOTH of these things aren't true, we know we have not yet found the beginning of the file type we are looking for, so keep running the loop. But if either one of them is true, then the condition is met and we break the loop.

5
  • My understanding of || is that as long as it matches one pattern it will return false whereas if I had used the && operator then it would need to satisfy both patterns. – Lucas Dec 4 '14 at 1:32
  • But you are using !=0. Try the && and see what happens. – lethaljd Dec 4 '14 at 1:35
  • yes it works now. I think I still have to weak my feof(file) though because I had to manually cut the program from running. I still don't understand why && should be used and not ||? Could you help? – Lucas Dec 4 '14 at 1:51
  • I edited my original answer to help. Please mark as accepted if that answers your question, or leave another comment if additional clarification is required. – lethaljd Dec 4 '14 at 21:01
  • thanks! I get it now. – Lucas Dec 5 '14 at 1:42

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .