Valgrind - 2 memory leaks | Recover.c | Pset-4

Question

Valgrind encountered two errors in the following code. I'm not able to get why is this happening even when I close files and free allocs. Please help if you can.

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

// Returns 1 if buffer is start of a new jpg file
int checkjpg(int buffer);

int main(int argc, char *argv[])
{
    if (argc == 1)
    {
        printf("Correct usage: ./recover file.raw");
        return 1;
    }
    FILE *f = fopen(argv[1], "r");
    if (f == NULL)
    {
        printf("Could not open %s.\n", argv[1]);
        return 1;
    }

    int *buffer[512];
    char name[8];
    int imgcount = 0;
    FILE *img = NULL;

    while (fread(buffer, 1, 512, f) == 512)
    {
        // If this is a jpg file AND its the first one
        if ((checkjpg(*buffer) == 1) && imgcount == 0)
        {
            imgcount++;
            sprintf(name, "%03i.jpg", imgcount - 1);
            img = fopen(name, "w");

            fwrite(&buffer, 1, 512, img);
        }
        else if (imgcount != 0)
        {
            // if new jpg file
            if (checkjpg(*buffer) == 1)
            {
                // close previous jpg and create next one
                fclose(img);
                imgcount++;

                sprintf(name, "%03i.jpg", imgcount - 1);
                img = fopen(name, "w");

                fwrite(&buffer, 1, 512, img);
            }
            else
            {
                // write to currently open jpg file
                fwrite(&buffer, 1, 512, img);
            }
        }
    }
    if (img != NULL)
    {
        fclose(img);
    }
    fclose(f);
}

int checkjpg(int buffer)
{
    if (buffer == 0xff && (buffer + 1) == 0x8d && (buffer + 2) == 0xff && ((buffer + 3) & 0xf0) == 0xe0)
    {
        return 1;
    }
    return 0;
}

According to Valgrind, these are the errors

==4023== Memcheck, a memory error detector
==4023== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==4023== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==4023== Command: ./recover card.raw
==4023== 
==4023== Invalid read of size 4
==4023==    at 0x4009CC: checkjpg (recover.c:67)
==4023==    by 0x400800: main (recover.c:29)
==4023==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==4023== 
==4023== 
==4023== Process terminating with default action of signal 11 (SIGSEGV)
==4023==  Access not within mapped region at address 0x0
==4023==    at 0x4009CC: checkjpg (recover.c:67)
==4023==    by 0x400800: main (recover.c:29)
==4023==  If you believe this happened as a result of a stack
==4023==  overflow in your program's main thread (unlikely but
==4023==  possible), you can try to increase the size of the
==4023==  main thread stack using the --main-stacksize= flag.
==4023==  The main thread stack size used in this run was 8388608.
==4023== 
==4023== HEAP SUMMARY:
==4023==     in use at exit: 552 bytes in 1 blocks
==4023==   total heap usage: 2 allocs, 1 frees, 4,648 bytes allocated
==4023== 
==4023== 552 bytes in 1 blocks are still reachable in loss record 1 of 1
==4023==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4023==    by 0x5694E49: __fopen_internal (iofopen.c:65)
==4023==    by 0x5694E49: fopen@@GLIBC_2.2.5 (iofopen.c:89)
==4023==    by 0x40077C: main (recover.c:14)
==4023== 
==4023== LEAK SUMMARY:
==4023==    definitely lost: 0 bytes in 0 blocks
==4023==    indirectly lost: 0 bytes in 0 blocks
==4023==      possibly lost: 0 bytes in 0 blocks
==4023==    still reachable: 552 bytes in 1 blocks
==4023==         suppressed: 0 bytes in 0 blocks
==4023== 
==4023== For counts of detected and suppressed errors, rerun with: -v
==4023== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
Segmentation fault

I'm closing the f file and freeing the buffer memory, still this problem.

Answer 1

Your code does not compile as written, but let's check the valgrind output anyway.

==4023== Invalid read of size 4
==4023==    at 0x4009CC: checkjpg (recover.c:67)
==4023==    by 0x400800: main (recover.c:29)
==4023==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

Address 0x0 is a red flag, it means you are trying to dereference a null pointer, i.e. an address that contains all zeros, pointing to an area of memory that is off-limits to C programs.

The problem seems to be here:

int *buffer[512];

When parsing C declarations, you must use the right-left rule.

1. Start at the centermost identifier: buffer
2. Look to the right, find an open bracket. Buffer is an array.
3. Keep looking right as long as you can. Buffer is an array of size 512, and then the declaration ends.
4. Look left, buffer is an array of 512 pointers.
5. Keep looking left as long as you can, buffer is an array of 512 pointers to int.

For more information about the C right-left rule, see: [http://cseweb.ucsd.edu/~ricko/rt_lt.rule.html]

It is likely that you intended buffer to be a simple array of 512 chars. In that case you might have written:

unsigned char buffer[512];

Try that, fix the compilation errors, and see how things work.

P.S. Review the line that checks the first 4 values in buffer. One of the values seems wrong. After I made these corrections, your code executes fine:

Valgrind was happy too:

Answer 2

This isn't complete code. Without seeing the rest, I can't be sure of what's happening, but I can make an pretty good guess.

The 552 bytes are left open because the last output file isn't closed before the program ends.

The 512 bytes are somehow related to the buffer. Since your checkjpg() function isn't posted, I suspect the problem lies there, or is at least related.

However, there's a simple fix to the problem. The buffer is of a known, fixed size and is reused throughout the program without having to change size or to have more memory allocated. Why create it using a pointer and malloc? Why not just create a 512 byte array and be done with it? Let the system take care of the overhead and the cleanup when the program shuts down.

If you want a more detailed answer, you'll need to add the missing code to the question in an edit.

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