c comparing file sizes using fstat issue

Question

My code comparing 2 file sizes seems to behave as if keyfile > sourcefile. Is there something missing from the code below and what can I change? The two files I'm using for testing are 3kb keyfile and 14kb sourcefile, which should activate the last if statement provided below.

/* Get size of sourcefile. */
if((sourcefile = fopen(argv[1], "rb"))== NULL)
{
printf("Can't open source file.\n");
printf("Please enter a valid filename.\n");
printf("USAGE: OTP <source file> <output file> <keyfile>\n");
return (1);
}

fflush(sourcefile);
fstat(fileno(sourcefile), &statbuf);
fclose(sourcefile);

/* Get size of keyfile. */
if((keyfile = fopen(argv[3], "rb"))== NULL)
{
printf("Can't open keyfile.\n");
printf("Please enter a valid filename.\n"); 
printf("USAGE: OTP <source file> <output file> <keyfile>\n");
return(1);

fflush(keyfile);
fstat(fileno(keyfile), &keybuf);
fclose(keyfile);
}

/* Open necessary files. */
keyfile=fopen(argv[3], "rb");
sourcefile=fopen(argv[1], "rb");
destfile=fopen(argv[2], "wb");

/* Check if keyfile is the same size as, or bigger than the sourcefile */
if((keybuf.st_size) < (statbuf.st_size))
{
printf("Source file is larger than keyfile.\n");
printf("This significantly reduces cryptographic strength.\n");
printf("Do you wish to continue? (Y/N)\n");
scanf("%c", &ans);
if(ans == 'n' || ans == 'N')
    {
    return (1);
    }
if(ans == 'y' || ans == 'Y')
    {
    printf("Proceeding with Encryption/Decryption.\n");
    }
}

ah yeah, you mean under /*opening necessary files*/? could do without the keyfile and source file there. edit coming up. — youjustreadthis, Oct 12 '12 at 20:11
Nonononono. Take out the `fclose`s before `/* opening necessary files */`. you don't need them since you just re-open the file anyway. Then remove the `fopen`s of keyfile and sourcefile under aforementioned comment. — slugonamission, Oct 12 '12 at 20:12
Also, you don't check if `destfile` could be opened. This will fail if the specified file couldn't be written to (permission errors etc). — slugonamission, Oct 12 '12 at 20:13
Capture the return value from fstat, and make an appropriate error message, if you get a negative value. And why call fflush() before fstat()? — , Oct 12 '12 at 20:15
The comment above the if where you compare the file sizes is the other way round than your actual test. — halex, Oct 12 '12 at 20:15
@halex - the OP is asking why it acts the opposite way (i.e. he's asking why it is acting as if keyfile.size > statbuf.size). — slugonamission, Oct 12 '12 at 20:20
ok, so my code wasn't as good as I thought haha. Would it be too much trouble to edit your answer with the changes to the code? I think I got it, but just to be sure you know:) I appologize for my newbieness but you've gotta learn somehow :) @slugonamission — youjustreadthis, Oct 12 '12 at 20:33
Since you actually tried to implement it yourself and want verification, sure. See my updated answer. Hope it reads alright. — slugonamission, Oct 12 '12 at 20:36
@Cœur this post is just one big typo, you should've cast CV instead — Antti Haapala -- Слава Україні, Apr 19 '19 at 09:29

slugonamission · Accepted Answer · 2012-10-12T20:35:20.333

It's because you only fstat the keyfile if it fails to open. Move your curly brace up. Also, as an optimisation of forms, don't open the file, stat it, close it and re-open it. Just don't close it and carry on.

So, since you asked

/* Get size of sourcefile. */
if((sourcefile = fopen(argv[1], "rb"))== NULL)
{
printf("Can't open source file.\n");
printf("Please enter a valid filename.\n");
printf("USAGE: OTP <source file> <output file> <keyfile>\n");
return (1);
}

fflush(sourcefile);
//fstat(fileno(sourcefile), &statbuf);   // <-- this is not needed
//fclose(sourcefile);                    // <-- this is not needed

/* Get size of keyfile. */
if((keyfile = fopen(argv[3], "rb"))== NULL)
{
printf("Can't open keyfile.\n");
printf("Please enter a valid filename.\n"); 
printf("USAGE: OTP <source file> <output file> <keyfile>\n");
return(1);
    }                                     // <-- this brace is new (well, moved) (1)

fflush(keyfile);
//fstat(fileno(keyfile), &keybuf);    // <-- not needed
//fclose(keyfile);                    // <-- not needed
//}                                     // <-- this brace has moved up 4 lines to (1)

/* Open necessary files. */
keyfile=fopen(argv[3], "rb");
sourcefile=fopen(argv[1], "rb");
destfile=fopen(argv[2], "wb");

/* Check if keyfile is the same size as, or bigger than the sourcefile */
if((keybuf.st_size) < (statbuf.st_size))
{
printf("Source file is larger than keyfile.\n");
printf("This significantly reduces cryptographic strength.\n");
printf("Do you wish to continue? (Y/N)\n");
scanf("%c", &ans);
if(ans == 'n' || ans == 'N')
    {
    return (1);
    }
if(ans == 'y' || ans == 'Y')
    {
    printf("Proceeding with Encryption/Decryption.\n");
    }
}

Also, you don't need to `fflush` it. You've not written anything to it. — slugonamission, Oct 12 '12 at 20:10
the internet should have more people like you haha. superhero(ish) =P — youjustreadthis, Oct 12 '12 at 20:36

c comparing file sizes using fstat issue

1 Answers1