2

My problem starts from the while loop. I have an if condition and inside of the if condition I create a file and write to it. But naturally, I can't use the pointer outside of the condition. I'm new to C and I'm looking for a way to make that pointer global variable like in python. Here is my code:

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

//define chunk size
const int chunksize = 512;

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

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

    //create a variable to store memory size
    fseek(inptr, 0L, SEEK_END);
    long int memorysize = ftell(inptr);
    rewind(inptr);

    //find how many chunk does memory card contain
    int nofchunks = memorysize / chunksize;

    //create a file counter
    int nofjpeg = 0;

    while(nofchunks > 0)
    {
        //create a temporary storage
        unsigned char chunk[chunksize];

        // read a 512 byte chunk from memory card
        fread(&chunk, chunksize, 1, inptr);
        
        FILE *outptr = NULL;

        //check the chunk if it is a JPEG by looking first 4byte of the chunk
        if (chunk[0] == 0xff && chunk[1] == 0xd8 && chunk[2] == 0xff && (chunk[3] & 0xf0) == 0xe0)
        {
            nofjpeg++;

            //create a temporary file name
            char filename[8];

            if (nofjpeg == 1)
            {
                //create a new file name
                sprintf(filename, "%03i.jpg", nofjpeg);

                //open the file in write mode
                outptr = fopen(filename, "w");
                if (outptr == NULL)
                {
                    fprintf(stderr, "Could not open %s.\n", argv[1]);
                    return 2;
                }
                fwrite(&chunk, chunksize, 1, outptr);
            }
            else
            {
                //close the previous file
                fclose(outptr);

                //create a new file name
                sprintf(filename, "%03i.jpg", nofjpeg);

                //open the file in write mode
                outptr = fopen(filename, "w");
                if (outptr == NULL)
                {
                    fprintf(stderr, "Could not open %s.\n", argv[1]);
                    return 2;
                }
                fwrite(&chunk, chunksize, 1, outptr);
            }

        }
        else if(nofjpeg > 1)
        {
        fwrite(&chunk, chunksize, 1, outptr);
        }
        nofchunks--;
    }
}

You can see that inside of the first inner if condition I open the file and write to it. And I need to close that file inside the following else condition. You can also see I also use that pointer following the outer if condition.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
Baran Aldemir
  • 71
  • 1
  • 7

2 Answers2

0

Declare the file pointer before the if statement but assign it a null value. It is then available after the if statement. Inside the if statement you can assign it a value which will persist after the if statement. Following the if statement you should check to make sure it's not null before de-referencing it.

//check the chunk if it is a JPEG by looking first 4byte of the chunk
FILE *outptr = (FILE*) NULL;   // Declare before the if and assign to NULL cast to type FILE*

if (chunk[0] == 0xff && chunk[1] == 0xd8 && chunk[2] == 0xff && (chunk[3] & 0xf0) == 0xe0)
{
    nofjpeg++;

    //create a temporary file name
    char filename[8];


    if (nofjpeg == 1)
    {
        //create a new file name
        sprintf(filename, "%03i.jpg", nofjpeg);

        //open the file in write mode
        outptr = fopen(filename, "w");   // Assign inside the if statement, but don't declare.
        if (outptr == NULL)
        {
            fprintf(stderr, "Could not open %s.\n", argv[1]);
            return 2;
        }
        fwrite(&chunk, chunksize, 1, outptr);
    }
    else
    {
        //close the previous file  ... this is likely a logical error; it should be NULL at this point, so there's nothing to close.
        if (outptr) fclose(outptr);   // Check to see if pointer is null before use.

        //create a new file name
        sprintf(filename, "%03i.jpg", nofjpeg);

        //open the file in write mode
        outptr = fopen(filename, "w");
        if (outptr == NULL)
        {
            fprintf(stderr, "Could not open %s.\n", argv[1]);
            return 2;
        }
        fwrite(&chunk, chunksize, 1, outptr);
    }

}
else if(nofjpeg > 1)
{
    // This is likely a logical error; outptr will be NULL here
    if (outptr) fwrite(&chunk, chunksize, 1, outptr);
}
nofchunks--;
andand
  • 17,134
  • 11
  • 53
  • 79
  • I tried that and I got this error: initializing 'FILE' (aka 'struct _IO_FILE') with an expression of incompatible type 'void *' FILE outptr = NULL; – Baran Aldemir Feb 08 '21 at 20:07
  • Edited... you should be able to cast it to a FILE* pointer. – andand Feb 08 '21 at 20:09
  • 2
    In C, casting `NULL` isn't necessary at all (in this context). The OP probably is using a C++ compiler to compile the C code. – M. Nejat Aydin Feb 08 '21 at 20:12
  • @andand Yeah, I forgot the star. Now I correct it but I got a new error: declaration shadows a local variable [-Werror,-Wshadow] FILE *outptr = fopen(filename, "w"); recover.c:43:15: note: previous declaration is here FILE *outptr = NULL; – Baran Aldemir Feb 08 '21 at 20:13
  • Right, you need to remove all of the declarations... but you likely have some logical errors remaining based on what I see in this code snippet. I made some additional changes as well. First you'll need to declare `outptr` before the first `if`, and then remove the declarations further down. – andand Feb 08 '21 at 20:14
  • I did declare `outptr` before the first `if`. And it says that declaration shadows the local `outptr`. @andand – Baran Aldemir Feb 08 '21 at 20:25
  • I'm using cloud based IDE. And not sure what compiler they are using. @M.NejatAydin – Baran Aldemir Feb 08 '21 at 20:31
  • 1
    @BaranAldemir right, shadowing means you're declaring a variable in an inner scope which hides another variable with the same name in an outer scope. To avoid this you need to remove the declaration `FILE* outptr = fopen(filename, "w");` so it becomes `outptr = fopen(filename, "w");` in both branches of your if statement. Beyond this, you may have some logical errors which I didn't address since you might have some reason for structuring things the way you do. – andand Feb 08 '21 at 20:31
  • If it were me, I'd open the file before the first `if` and check that the file actually openned. Then, use `outptr` to perform the `fwrite` operations in your nested `if` statements and then close it after the end of the outermost `if`. If doing this, I would remove the `fopen` statements inside the `if` statements. This would make the code more clear and remove what appear to be the logical errors in your code. – andand Feb 08 '21 at 20:37
  • @andand Thanks, I guess it works now. I just get a Segmentation fault but I think it has nothing to do with our problem. – Baran Aldemir Feb 08 '21 at 20:38
  • @BaranAldemir one reason for a segfault is dereferencing a null pointer. It's not the only reason, but it's a common one. Check to see if `outptr` is null before using it. – andand Feb 08 '21 at 20:40
  • @andand Yeah, it is null because we initialized `outptr` with `null`.Sorry, if I keep asking stupid questions. It's just I'm a little confused. I guess I need to use malloc() instead. – Baran Aldemir Feb 08 '21 at 21:04
  • @BaranAldemir I wouldn't use `malloc`. I probably need additional context. If this code is inside a loop, please edit your question to show how you are controlling the loop. Is it a while loop or something else? You can probably simplify the structure to make things easier to follow. – andand Feb 08 '21 at 21:41
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/228436/discussion-between-baran-aldemir-and-andand). – Baran Aldemir Feb 08 '21 at 22:09
0

Finally found the solution!!! (I am using cloud visual studio)

Outside the loop declare it like this: FILE * outptr = (FILE *) outptr ;

And then just use outptr anywhere else (inside the differnt IF statements):

..inside the 1st IF >>>

  • open the file with: outptr = fopen(filename, "w");
  • write: fwrite(&chunk, chunksize, 1, outptr);

..inside the last ELSE IF >>>

  • write: fwrite(&chunk, chunksize, 1, outptr);
Tobaton
  • 1
  • 1