Anonymous
Anonymous

Reputation: 19

Concatenating files using read and write system calls

I wrote this simple code to concatenate two files. Why doesn't this work? Please help.

int main(int arg, char* argv[]){  
    int fd1, fd2;  
    char c;  
    fd1 = open(argv[1], O_APPEND);  
    fd2 = open(argv[2], O_RDONLY);  
    while(read(fd2, &c, sizeof(c))) {  
        write(fd1, &c, sizeof(c));  
    }  
    close(fd1);  
    close(fd2);  
    return 0;  
}  

Upvotes: 1

Views: 799

Answers (1)

Jonathan Leffler
Jonathan Leffler

Reputation: 754090

As I noted in a comment:

You need to test the results of open() and write(). Your call with O_APPEND needs O_WRONLY too. And if you want it to create the file too, you need various other options too — O_CREAT; maybe O_TRUNC but probably not; maybe O_EXCL; maybe some others too — and you need a mode argument for the third argument (e.g. 0644 — owner can read or write, group and others can only read; or something more restrictive).

You also need to check that you have 2 file name arguments.

Fixing those issues leads to code similar to this:

int main(int arg, char* argv[]){  
    if (argc < 3)
    {
        fprintf(stderr, "Usage: %s out-file in-file\n", argv[0]);
        return 1;
    }
    int fd1 = open(argv[1], O_APPEND|O_WRONLY|O_CREAT, 0644);
    if (fd1 < 0)
    {
        fprintf(stderr, "%s: failed to open/create file %s for writing\n", argv[0], argv[1]);
        return 1;
    }
    int fd2 = open(argv[2], O_RDONLY);  
    if (fd2 < 0)
    {
        fprintf(stderr, "%s: failed to open file %s for reading\n", argv[0], argv[2]);
        return 1;
    }
    char c;
    while (read(fd2, &c, sizeof(c)) == sizeof(c)) {  
        if (write(fd1, &c, sizeof(c)) != sizeof(c))
            break;  
    }  
    close(fd1);  
    close(fd2);  
    return 0;  
}

I note that you might get warnings about comparing signed and unsigned values with the read() and write() tests — the functions return a ssize_t and sizeof produces a size_t (2 vs 1 letters s). You can cast around that, but it's ugly. The code is also woefully inefficient. Single character reads and writes work, but you get dramatically better performance in general with buffer sizes in the 1 KiB to 4 KiB range, and even bigger. Of course, then you have to trap and use the value returned by read() so that the correct amount of data is written.

Note that the original code would continue the loop if the read() failed (returned -1). It would only stop if the read() was successful at returning 0 when it reached EOF.

Upvotes: 1

Related Questions