Problem in code with File Descriptors. C (Linux)

Question

I've written code that should ideally take in data from one document, encrypt it and save it in another document.

But when I try executing the code it does not put the encrypted data in the new file. It just leaves it blank. Someone please spot what's missing in the code. I tried but I couldn't figure it out.

I think there is something wrong with the read/write function, or maybe I'm implementing the do-while loop incorrectly.

#include <stdio.h>
#include <stdlib.h>
#include <termios.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <string.h>
#include <unistd.h>


int main (int argc, char* argv[]) 
{
    int fdin,fdout,n,i,fd;
    char* buf;
    struct stat fs;

    if(argc<3)
        printf("USAGE: %s source-file target-file.\n",argv[0]);

    fdin=open(argv[1], O_RDONLY);
    if(fdin==-1)
        printf("ERROR: Cannot open %s.\n",argv[1]);

    fdout=open(argv[2], O_WRONLY | O_CREAT | O_EXCL, 0644);
    if(fdout==-1)
        printf("ERROR: %s already exists.\n",argv[2]);

    fstat(fd, &fs);
    n= fs.st_size;
    buf=malloc(n);

    do
    {
        n=read(fd, buf, 10);

        for(i=0;i<n;i++)
            buf[i] ^= '#';

        write(fd, buf, n);
    } while(n==10);

    close(fdin);
    close(fdout);
}

Answer 1

You are using fd instead of fdin in fstat, read and write system calls. fd is an uninitialized variable.

Answer 2

// Here...
fstat(fd, &fs);

// And here...
n=read(fd, buf, 10);

for(i=0;i<n;i++)
    buf[i] ^= '#';

write(fd, buf, n);

You're reading and writing to fd instead of fdin and fdout. Make sure you enable all warnings your compiler will emit (e.g. use gcc -Wall -Wextra -pedantic). It will warn you about the use of an uninitialized variable if you let it.

Also, if you checked the return codes of fstat(), read(), or write(), you'd likely have gotten errors from using an invalid file descriptor. They are most likely erroring out with EINVAL (invalid argument) errors.

fstat(fd, &fs);
n= fs.st_size;
buf=malloc(n);

And since we're here: allocating enough memory to hold the entire file is unnecessary. You're only reading 10 bytes at a time in your loop, so you really only need a 10-byte buffer. You could skip the fstat() entirely.

// Just allocate 10 bytes.
buf = malloc(10);

// Or heck, skip the malloc() too! Change "char *buf" to:
char buf[10];

Answer 3

All said it true, one more tip.

You should use a larger buffer that fits the system hard disk blocks, usually 8192. This will increase your program speed significantly as you will have less access to the disk by a factor of 800. As you know, accessing to disk is very expensive in terms of time.

Another option is use stdio functions fread, fwrite, etc, which already takes care of buffering, still you'll have the function call overhead. Roni