read() not working as expected. Returns value 1 on successful read of large block

Question

I am developing a process that, among other things, will copy files. I've run across a problem that the read function does not work as I expected.

My research and my prior experience tell me that the (fcntl) read() function is supposed to return number of bytes read, but it's returning 1 instead. I've reached my limit of things to try (playing mostly with varying the buffer size from 16 to 16MB). No matter what I try, I always end up with either a segmentation fault (buffer size 16MB) or read() returns a value of 1.

To demonstrate this, I've reduced the program to a bare minimum that still compiles and execute, but doesn't do what's needed. To get good performance, I need a nice large buffer (preferably 15MB but 1MB would be OK in a grind). In this application, with a buffer size of 1M, the output filesize ends up being 1/1048567 times the original size (so a 16MB input file is copied to 16 bytes). Yes, I could go to a buffer size of 1, but as stated, this should be more efficient than that.

This can be tested on your system by setting "in" to some existing file on your system, and "out" to a filename that would also be valid. NOTE: "out" will be overwritten if it exists!!!

Environment is Cygwin 3.2.0(0.340/5/3)

Any clues what I need to do to make this work as documented?

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>

#define BOOL char
#define TRUE (1 < 2)
#define FALSE (1 > 2)
#define MAX_BUFFER_SIZE 1048576 // 1M.  16MB (16777216) will caus segmentation fault

int    copyfile(const char *in, const char *out) ;
BOOL debug = TRUE ;


int main(int argc, char *argv[]) {
    int rc = 0 ;
    char *in = "/e/DCIM/100MEDIA/DJI_0292.MP4" ;
    char *out = "/m/Videos/2021/2021-08-Other/temp/08140708_dji_0292.mp4" ;

    rc = copyfile(in, out) ;
    return rc ;
    }



int copyfile(const char *in, const char *out) {
    int rtnVal = TRUE ;
    int fd_in, fd_out ;
    char buff[MAX_BUFFER_SIZE], *out_ptr ;
    ssize_t nRead, nRemaining, nWritten, totWritten = 0 ;
    int save_errno ;
    struct stat st ;

    unlink(out) ; // In case it exists ...
    if ((fd_in = open(in, O_RDONLY)) < 0) {
        fprintf(stderr, "Could not open input file %s\n\t%s\n", in, strerror(errno)) ;
        rtnVal = FALSE ;
        }
    else if ((fd_out = open(out, O_WRONLY | O_CREAT | O_EXCL, 0666)) < 0) {
        fprintf(stderr, "Could not open output file %s\n\t%s\n", out, strerror(errno)) ;
        rtnVal = FALSE ;
        }
    if (rtnVal) {
        if (stat(in, &st) < 0) {
            fprintf(stderr, "Could not stat input file: %s\n\t%s\n", in, strerror(errno)) ;
            exit(1) ;
            }
        if (debug)
            fprintf(stdout, "\n\n\nSize of %s is %ld\n", in, st.st_size) ;
        while (((nRead = read(fd_in, buff, MAX_BUFFER_SIZE) > 0) && (rtnVal == TRUE))) {
            // if (debug)
            //     fprintf(stdout, "%ld bytes read this time.\n", nRead) ;
            out_ptr = buff ;
            nRemaining = nRead ;
            while (nRemaining > 0) {
                nWritten = write(fd_out, out_ptr, nRemaining) ;
                if (nWritten >= 0) {
                    out_ptr += nWritten ;
                    totWritten += nWritten ;
                    nRemaining -= nWritten ;
                    if (debug)
                        fprintf(stdout, "\tDEBUG: %ld bytes read. So far, %ld bytes have been written. %ld remain\n"
                                      , nRead, totWritten, nRemaining) ;
                    }
                else if (errno != EINTR) { // No error - try again (see https://stackoverflow.com/questions/41474299/checking-if-errno-eintr-what-does-it-mean)
                    rtnVal = FALSE ;
                    fprintf(stderr, "Could not copy from: %s\n"
                                    "                 to: %s\n"
                                    "     %s\n"
                                  , in, out, strerror(errno)
                                   ) ;
                    break ;
                    }
                } // While remaining
            }     // While read()
        }         // If rtnVal
    save_errno = errno ;
    if (fd_in)  close(fd_in) ;
    if (fd_out) close(fd_out) ;

    if (stat(out, &st) < 0) {
        fprintf(stderr, "Could not stat output file: %s\n\t%s\n", in, strerror(errno)) ;
        exit(1) ;
        }
    if (debug)
        fprintf(stdout, "Size of %s is %ld\n", out, st.st_size) ;
    errno = save_errno ;
    return(rtnVal) ;
    }

sample run:

Size of /e/DCIM/100MEDIA/DJI_0292.MP4 is 406514367
    DEBUG: 1 bytes read. So far, 1 bytes have been written. 0 remain
    DEBUG: 1 bytes read. So far, 2 bytes have been written. 0 remain
    DEBUG: 1 bytes read. So far, 3 bytes have been written. 0 remain
    DEBUG: 1 bytes read. So far, 4 bytes have been written. 0 remain
    DEBUG: 1 bytes read. So far, 5 bytes have been written. 0 remain
    DEBUG: 1 bytes read. So far, 6 bytes have been written. 0 remain
    DEBUG: 1 bytes read. So far, 7 bytes have been written. 0 remain
...
...
    DEBUG: 1 bytes read. So far, 385 bytes have been written. 0 remain
    DEBUG: 1 bytes read. So far, 386 bytes have been written. 0 remain
    DEBUG: 1 bytes read. So far, 387 bytes have been written. 0 remain
    DEBUG: 1 bytes read. So far, 388 bytes have been written. 0 remain
Size of /m/Videos/2021/2021-08-Other/temp/08140708_dji_0292.mp4 is 388

Barmar · Accepted Answer · 2021-09-03T21:30:20.897

1

You're setting nread to the result of the > comparison operation, not the result of read(), because > has higher precedence than =. You need parentheses around the assignment.

while ((nRead = read(fd_in, buff, MAX_BUFFER_SIZE)) > 0 && rtnVal) {

edited Sep 03 '21 at 21:30

answered Sep 03 '21 at 21:01

Barmar

741,623
53
500
612

I realize the redundancy in this code. As I said, this is a subset of a larger puzzle. The && is needed in the real code. Having said that, the parenetheses are properly placed, seems to me. – Dennis Sep 03 '21 at 21:23
You don't have any parentheses around the assignment. You have `(nRead = read(...) > 0)` – Barmar Sep 03 '21 at 21:27
oops! I’m particularly dense today. I saw your code, and took it as mine. Thanks! – Dennis Sep 03 '21 at 21:28
I added back the `rtnVal` check. – Barmar Sep 03 '21 at 21:30
1

You don't need `== TRUE` if it can only contain true or false. – Barmar Sep 03 '21 at 21:30
Some things I do for human readability. Thanks again for your help. – Dennis Sep 03 '21 at 21:36
Experienced programmers avoid it. One reason is that it's too easy to typo as `rtnVal = TRUE` – Barmar Sep 03 '21 at 21:38
Guess my 45+ years wasn't enough. Regarding the typo agreed it is easy to do. I didn't. 'nuff said. I'll accept this answer when the system lets me. – Dennis Sep 03 '21 at 21:39
@Dennis in 45y you did not heard about `stdbool`? Comparing to the TRUE defined this way is a very bad practice. Why? Because most of the C functions return non zero fir the true (like isalpha for example), not 0 or 1 as your definition of TRUE and FALSE. – 0___________ Sep 03 '21 at 22:08
I don’t believe I said that. stdbool was introduced in ‘99, about 15 years after my own standard was ingrained in my soul. I dislike stdbool since it strays from the convention that constants should have UPPEE names. – Dennis Sep 03 '21 at 22:24
Your reason for saying that TRUE is a bad idea because there are other ways to express "not false." We seem determined to disagree, and we’ve strayed far from the topic, which had little to do with boolens and the like. Can we please abandon this brick-over-the-wll battle? Thank you, there will be no more from me here. – Dennis Sep 03 '21 at 22:28

read() not working as expected. Returns value 1 on successful read of large block

1 Answers1