Pset 4 Recover: Image pointer returns null on second iteration of code

Question

For some reason my code seems to be either unassigning the pointer to my image when it reaches the second iteration following the opening of the first jpg file. (I know this because there is a obviously corrupt JPG in the folder where the program is being executed.) I inserted two printf functions to see what exactly is going on and have found two strange things. The variable I've instituted to check for the jpeg magic number returns "bit." which I do not understand, and that right before the program seg-faults (as pointed out in my code by a comment), the second printf shows that the pointer to the image file where the bits should be written to is (NULL) despite it having to have been set in order for that part of the loop to take place. (GDB has also been utterly unhelpful in ascertaining what the issue is due to it spending an incomprehensible amount of cycles "in dl-addr.c", making finding the precise route of the issue frustratingly difficult.

void* buffer = malloc(64);
void* mnh = malloc(4);
int imageno = 1;
int imgopen = 0;
void* img = NULL;
char* title = malloc(7*8);
char* jpegmn1 = "0xff0xd80xff0xe0";
char* jpegmn2 = "0xff0xd80xff0xe1";

// init read loop and end condition for program
while (fread(buffer, 64, 1, card) == 1)
{
    // Look for JPEG magic numbers
    fseek(card, -64, SEEK_CUR);
    fread(mnh, 4, 1, card);
    fseek(card, 60, SEEK_CUR);
    printf("%s\n", mnh); //DEBUG CODE

    // IS THIS THE START OF A NEW JPG?
    //Yes, new jpg
    if (strcmp(mnh, jpegmn1) == 1 || strcmp(mnh, jpegmn2) == 1)
    {
        //DO YOU HAVE AN IMAGE OPEN?
        //Yes, image open
        if (imgopen == 1)
        {
            // generate file name
            if (imageno > 9)
                sprintf(title, "0%d.jpg", imageno);
            else
                sprintf(title, "00%d.jpg", imageno);

            // close old image, open new image, iterate title generator
            fclose(img);
            FILE* img = fopen(title, "a");
            imageno++;
            imgopen = 1;
            if (img == NULL)
            {
                fclose(img);
                fprintf(stderr, "Could not create %s.\n", title);
                return 3;
            }

            // Write buffer to file
            fwrite(buffer, 64, 1, img);
         }  
         //No, image not open
         else
         {

            // generate file name
            if (imageno > 9)
                sprintf(title, "0%d.jpg", imageno);
            else
                sprintf(title, "00%d.jpg", imageno); 

            // open new image, iterate title
            FILE* img = fopen(title, "a");
            imageno++;
            imgopen = 1;

            // Write buffer to file
            fwrite(buffer, 64, 1, img);
         }
    }
    //No, not new jpg
    else
    {
        //DO YOU HAVE AN IMAGE OPEN?
        //Yes, image open
        if (imgopen == 1)
        {
            // Write buffer to file
            printf("%s\n", img); //DEBUG CODE
            fwrite(buffer, 64, 1, img); //VALGRIND SAYS THIS LINE CRASHES THE PROGRAM
        }
        //No, image not open, useless data
        else
        {
            // Do nothing, allow iterate to next 512 bit segment
        }
    } 
}

// write leftover data to final image
fwrite(buffer, 64, 1, img);

// close infile
fclose(card);

// close outfile
fclose(img);

// free memory
free(buffer);
free(mnh);
free(title);

Answer 1

Malloc returns a pointer to void. However, the pointer variable to which you are assigning the aforementioned value, buffer for instance, should not be declared as a void data type. The call to malloc should be typecast to the same data type. With regard to the initial declaration of img: it should be a FILE pointer.

Answer 2

Respectfully, you should reconsider the size of your buffer and the amount of bytes you fread. Also, the requested memory for the character pointer named 'title' (7*8) is not appropriate. The logic behind the latter statement seems to be that you wanted seven characters' worth of memory. If so, you have not taken the unit that is used by malloc into account. For the sake of simplicity, you may have taken the usual amount of memory required for a char as given - although simplicity is not the first thing that comes to mind when viewing your approach. The sooner you get in the habit of rendering this part of your program machine independent with sizeof() the better. Indeed, in this case it would have helped in getting the correct number of bytes. You also do not need the branches for your sprintf assignments to title. Just use the right width specifier in addition to the zero fill flag to your conversion character. In general your approach seems unnecessarily complex. It is not only less efficient, but it has engendered a number of additional errors.