Copying one file to another(Unix/C)?

Question

I have written the following code to copy one file to another. Although the code works, the code still prints both error messages. Why is this ? I am a complete beginner to Unix and C programming(although I have worked with C++ before), so any help in as much detail as possible would be great. Thanks !

int main(int argc, char *argv[])
{
    int n;
    char buf[4096];
    while ((n=read( open(argv[1], O_RDONLY) , buf, 4096))>0)
    {
        if (write(creat(argv[2], S_IREAD | S_IWRITE ), buf, n)!=n)
            printf("Error writing to file.\n");
    }
    if (n<0)
        printf("Error reading from file.\n");
    exit(0);
}

You re-open both the files with every iteration of the loop. — Weather Vane, Jan 31 '16 at 21:53
*"The code works"* but did you actually check that the destination file has the same content as the source file? Please test with file size > 4096. — Weather Vane, Jan 31 '16 at 21:59
What you you think where reading a newly opened file starts? And What the `creat`e fucntion does if the file already eists? — too honest for this site, Jan 31 '16 at 22:05
`read()` and `write()` return `ssize_t`, not `int`. They are *not* the same thing. — Andrew Henle, Jan 31 '16 at 22:13
@WeatherVane Yes, after I run the code, the destination has the same content as the original file. — Jenna Maiz, Jan 31 '16 at 22:17
@bayesianStudent it cannot be, if the file size is > 4096. If your second error message was printed, the `read` has failed, and the `write` never happens. — Weather Vane, Jan 31 '16 at 22:29

P.P · Answer 1 · 2016-01-31T22:15:45.347

You are opening the file in each iteration and attempt to creat the file in each iteration.

So except the very first iteration, all subsequent writes will fail. It probably "seems to work" because your input file contains less than 4096 bytes. So the first call to write was to make it look like everything was copied. If use an input which has more than 4096 bytes, you'll see only the first 4096 bytes (assuming both read() and write() don't fail).

If write() were to succeed all the time (e.g. you had the creat() outside the loop) then the open() call continuously opens the same file and is potentially an infinite loop or your system would run out of file descriptor and return an invalid file descriptor and read() will fail on that.

Long story short: don't write code like that :)

Move both calls to open() and creat() outside the loop:

int fd = open(argv[1], O_RDONLY);
if (fd == -1) { 
   perror("open");
   exit(1);
}

int fd2 = creat(argv[2], S_IREAD | S_IWRITE );
if (fd2 == -1) { 
   perror("write");
   exit(1);
}

while ( (n=read( fd , buf, 4096)) > 0 )
{
    if ( write(fd2 , buf, n) != n )
        printf("Error writing to file.\n");
}

Also, add some error checking. Check that the fd's (File Descriptors) are not negative. The file paths, via the command line, could be invalid. — BryanT, Jan 31 '16 at 22:12

score 2 · Answer 2 · answered Feb 01 '16 at 00:02

The above answer has spotted on however one should refrain from using creat(), as create() is an obsolete function. create(filename, mode); is equivalent to open(filename, O_WRONLY | O_CREAT | O_TRUNC, mode);

http://www.gnu.org/software/libc/manual/html_node/Opening-and-Closing-Files.html

The code could be better written as:

#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
#include <sys/types.h>
#include <sys/stat.h>

#define BUFSIZE 4096

int main(int argc, char **argv)
{
    int infd, outfd;
    int n;
    char *infile = NULL;
    char *outfile = NULL;
    int src_flags, dest_flags;
    mode_t dest_perms;
    char buf[BUFSIZE];

    if (argc < 3) {
        fprintf(stderr, "At least src and dest files must be specified\n"); /*Never rely on users' inputs*/
        /*Print usage information if possible*/
        exit(EXIT_FAILURE); 
    }

    infile = argv[1];
    outfile = argv[2];  /*Presuming the order*/            

    src_flags = O_RDONLY;
    dest_flags = O_CREAT | O_WRONLY | O_TRUNC;  
    /*creat() is equivalent to open(fname, O_CREAT | O_WRONLY | O_TRUNC, mode)*/
    dest_perms = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH; /*rw-rw-rw-*/

    infd = open(infile, src_flags);
    if (infd == -1) {
        perror("cannot open src ");
        exit(EXIT_FAILURE);
    }

    outfd = open(outfile, dest_flags, dest_perms);
    if (outfd == -1) {
        perror("cannot open dest ");
        exit(EXIT_FAILURE);
    }

    while ((n = read(infd, buf, BUFSIZE)) > 0) {
        if (write(outfd, buf, n) != n) {
            fprintf(stderr, "failed to write buf\n");
            goto exit_failure;
        }
    }

    if (n == -1) {
        fprintf(stderr, "read() failed\n");
        goto exit_failure;
    }

exit_failure:
     if (infd) {
        close(infd);
     }

    if (outfd) {
        close(outfd);
    }

    printf("Copy successful\n");

    exit(EXIT_SUCCESS);
}

Copying one file to another(Unix/C)?

2 Answers2

Linked