Copy content from one file to another in C-CodePudding

Given one txt file Im trying to make a C program that copy the content of that into other which name is passed by arguments.The program must read blocks of 512 bytes from the source file and write the bytes read in the destination file.

My attempt:

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

int main(int argc, char* argv[]){
    if(argc < 2){
        printf("ERROR: Missing arguments\n");
        exit(1);    
    }

    int fdo, fdd;

    if((fdo = open(argv[1], O_RDONLY)) == -1){
        printf("ERROR: Origin file %s can not be opened\n", argv[1]);
        exit(1);
    }

    if(fdd = open(argv[2], O_CREAT | O_TRUNC, 0666) == -1){
        printf("ERROR: Dest. file %s can not be opened\n", argv[2]);
        exit(1);
    } 

    char buff[512];
    size_t n_bytes;

    while((n_bytes = read(fdo,buff,512)) > 0){
        if(write(fdd,buff,n_bytes) < 0){
            printf("Can not write buffer content in %s \n", argv[2]);
            exit(1);
        }
    }

    if(n_bytes < 0){
        printf("Can not read %s file \n", argv[1]);
        exit(1);
    }

    close(fdo);
    close(fdd);

    return 0;
}

The content of the file test.txt is:

abcdef
1890

And their permissions are:

usuarioso@usuarioso-virtualbox:~/Documentos/SO 2022/pr2/API de ficheros y directorios$ ls -l
total 32
-rwxrwxr-x 1 usuarioso usuarioso 17048 nov 23 16:15 copyrf
-rw-rw-r-- 1 usuarioso usuarioso   774 nov 23 16:39 copyrf.c
-rw-rw-r-- 1 usuarioso usuarioso    12 nov 23 16:52 test.txt

However when I execute it I get the following:

usuarioso@usuarioso-virtualbox:~/Documentos/SO 2022/pr2/API de ficheros y directorios$ gcc -o copyrf copyrf.c
usuarioso@usuarioso-virtualbox:~/Documentos/SO 2022/pr2/API de ficheros y directorios$ ./copyrf test.txt test1.txt
abcdef
1890
usuarioso@usuarioso-virtualbox:~/Documentos/SO 2022/pr2/API de ficheros y directorios$ ls -l
total 28
-rwxrwxr-x 1 usuarioso usuarioso 17008 nov 23 17:00 copyrf
-rw-rw-r-- 1 usuarioso usuarioso   771 nov 23 16:59 copyrf.c
-rw-rw-rw- 1 usuarioso usuarioso     0 nov 23 17:00 test1.txt
-rw-rw-r-- 1 usuarioso usuarioso    12 nov 23 16:52 test.txt
usuarioso@usuarioso-virtualbox:~/Documentos/SO 2022/pr2/API de ficheros y directorios$ cat test1.txt
usuarioso@usuarioso-virtualbox:~/Documentos/SO 2022/pr2/API de ficheros y directorios$

i.e the file test1.txt is created but is empty and the content of the file test.txt is printed in console.

What am I missing?

CodePudding user response：

Clang's gcc points out the problem pretty well:

$ gcc temp.c
temp.c:19:12: warning: using the result of an assignment as a condition without parentheses [-Wparentheses]
    if(fdd = open(argv[2], O_CREAT | O_TRUNC, 0666) == -1){
       ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
temp.c:19:12: note: place parentheses around the assignment to silence this warning
    if(fdd = open(argv[2], O_CREAT | O_TRUNC, 0666) == -1){
           ^
       (                                                 )
temp.c:19:12: note: use '==' to turn this assignment into an equality comparison
    if(fdd = open(argv[2], O_CREAT | O_TRUNC, 0666) == -1){
       ^
           ==

... in particular, fdd is being set to the result of (open(...) == -1), i.e. to 1 if the open() call returned -1, or to 0 if the open() call returned any other value. Since neither 0 nor 1 is the value of the file-descriptor returned by open(), your subsequent calls to write(fdd, ...) are using an invalid file-descriptor value and therefore failing.

The fix would be to add parentheses as you did for your first/open-for-read open() call, making it like this instead:

if((fdd = open(argv[2], O_CREAT | O_TRUNC, 0666)) == -1){

In addition to that, you need to add O_WRONLY to the oflag argument of your second call to open(), otherwise fdd will not be allowed to write to the file.

CodePudding user response：

You forgot some parentheses so fdd is getting set to 0 or 1 instead of the return value of open. Compare it to the fdo code. It's best to avoid assignment inside an expression like that unless you really know what you're doing. It would also be one of the first things to remove while simplifying your code to make a minimal complete verifiable example for Stack Overflow.

CodePudding user response：

In the line:

if((fdo = open(argv[1], O_RDONLY)) == -1)

you put the assignment into brackets. This first assigns the opened file descriptor to fdo and then checks if that value is equal to -1 afterwards.

On the other hand, in second case:

if(fdd = open(argv[2], O_CREAT | O_TRUNC, 0666) == -1)

you did not put the assignment in the brackets. Because of this, the == operator is evaluated first, which returns a value of 0 (false) or 1 (true) instead of a valid file descriptor. This value is then assigned to fdd, causing you to write to a wrong file descriptor (standard output).