Low-level C I/O: When read from one file and write to another I'm hitting an infinite loop

Question

I am working on an assignment that only allows use of low-level I/O (read(), write(), lseek()) as well as perror().

I have been able to open the nessisary in and out files with correct permissions, but when I output I get an infinite loop of the in file contents to out. See snippet below...

void *buf = malloc(1024);
while((n = read(in, buf, 1024)) > 0){
    if(lseek(in, n, SEEK_CUR) == -1){
        perror("in file not seekable");
        exit(-1);
    }
    while((m = write(out, buf, n)) > 0){
        if(lseek(out, m, SEEK_CUR) == -1){
            perror("out file not seekable");
            exit(-1);
        }
    }
    if(m == -1){ perror("error writing out"); exit(-1); }
}
if(n == -1){ perror("error reading in"); exit(-1); }

I have removed some error trapping from my code and you can assume the variables are initialized and includes statements are there.

Answer 1

Problem is the inner loop:

while((m = write(out, buf, n)) > 0){

should really be

if((m = write(out, buf, n)) > 0){

You only want buf to be written once, not infinitely many times. What you also need to handle is short writes, that is, when write returns with m < n && m > 0.

Also, the lseek() calls are wrong, but they don't lead to the loop. read() and write() already advance the current file offset. You do not need to manually advance it, unless you want to skip bytes in the input or output file (note that in the output file case, on UNIX, skipping bytes may lead to so-called "holes" in files, regions which are zero but don't occupy disk space).

Answer 2

Why are you seeking on the input file after your read? Since you will, at most, read 1024 bytes (meaning n will be somewhere between 0 and 1024), you will be continuously seeking to somewhere beyond where you've left the input file pointer so that you'll lose data in the transfer (including probably beyond the end of the file when you get near the end).

This may be one cause why you have an infinite loop but the far more insidious one is the use of while for the write. Since this will return values greater than zero on success, you will continuously write the first chunk to the file over and over. At least until you run out of disk space or other resources.

You also don't need the seek on the write either. The read and write calls do what they have to do and advance the file pointer correctly for the next read or write - it's not something you have to do manually.

You can probably simplify the whole thing to:

while ((n = read (in, buf, 1024)) > 0) {
    if ((m = write (out, buf, n)) != n) {
        perror ("error writing out");
        exit (-1);
    }
}

which has the advantages of:

• getting rid of the seek calls;
• removing the 'infinite' loop;
• checking that you've written all the bytes requested.