1

First time poster here. Cannot figure out why I can't recover this one image. And any time I make changes to the code to get at it, I end up losing all the images. Please help.

// Recovers jpeg images from a memory card

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

typedef uint8_t BYTE;
BYTE buffer[512];

int main(int argc, char *argv[])
{
    // Ensure proper usage
    if (argc != 2)
    {
        fprintf(stderr, "Usage: ./recover images\n");
        return 1;
    }

    // Remember filenames
    char *infile = argv[1];
    char outfile[8];

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

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

    // Initialize output file
    outptr = NULL;

    // Initialize image counter
    int n = 0;

    // Identify JPEGs, reading through memory card file
    while (fread(buffer, sizeof(buffer), 1, inptr) == 1)
    {
        // Check if start of a new JPEG
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
        {
            // If we've found our first JPEG, open outfile for writing
            if (n == 0)
            {
                outptr = fopen(outfile, "w");
                n++;
            }
            // If NOT our first JPEG, close previous image & open new one
            else if (n > 0)
            {
                fclose(outptr);
                outptr = fopen(outfile, "w");
                n++;
            }
            // Write new JPEG to outfile, 512 bytes at a time
            fwrite(buffer, sizeof(buffer), 1, outptr);
            sprintf(outfile, "%03d.jpg", n);
        }
        // Else if NOT start of a new JPEG
        else
        {
            if (n > 0)
            {
                fwrite(buffer, sizeof(buffer), 1, outptr);
            }
        }
    }
    // Close files
    fclose(outptr);

    fclose(inptr);

    // Success
    return 0;
}
3
  • Hi, and welcome to StackExchange. Unfortunately, the first part of your code is hard to read. Could you please edit your question so that all of your code shows up within a code block? Sep 24 '18 at 14:49
  • The first time you try to open the outputfile, its name (outfile) has not yet been initialised. Sep 24 '18 at 19:08
  • Thanks, Peter, for the warm welcome and your response! This was a key part of the problem for sure. I finally got the code to work. =)
    – ChoLoo
    Sep 25 '18 at 4:43
2

The first file is being opened, but it isn't where you think it is.

The code opens the first output file before the start of the for loop, but there's a serious problem! Examine this block of code (unrelated code removed).

// Remember filenames
char *infile = argv[1];
char outfile[8];
...

// Open output file (outfile)
FILE *outptr = fopen(outfile, "w");
...

// Initialize output file
outptr = NULL;
...
 while (fread(buffer, sizeof(buffer), 1, inptr) == 1)
    {
        // Check if start of a new JPEG
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
        {
            // If we've found our first JPEG, open outfile for writing
            if (n == 0)
            {
                outptr = fopen(outfile, "w");
                n++;
            }

This opens a file using the string contained in outfile as the file name. Unfortunately, outfile is never initialized. It contains random data, so the file created will have a random name. If you list all the files in the directory, there will probably be a file with an unrecognized name. If you rename it to xxx.jpg (replace the whole file name, it may contain non-printable characters), you'll find your missing file.

If outfile were initialized, this would work. Having said that, the handling of the first file is kind of kludgy. All the needed code is present in the code that handles the later files. Try rethinking it to handle the first file too.

There's another reason to do this. What if you were writing this program as a tool to use regularly. If you were processing an input file that didn't have any image files inside, this code would always create at least one file. That file would be junk left on the hard drive. Not terribly kind to the user, is it? ;-)

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)

1
  • Cliff B, whoever & wherever you are, thank you a million times over. You not only helped me get the code to work, you also helped me understand a pretty basic but key concept. I'm just sorry that I put off posting for help for as long as I did. Again, thank you so much!
    – ChoLoo
    Sep 25 '18 at 4:41

You must log in to answer this question.

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