read() and buffer size errors in (C Linux)

Question

//EDIT: I set the first handle's flag as O_WRONLY, it should be O_RDONLY and that was causing a problem.

I'm working on a simple program in Linux using C that would copy text from one file to another.

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

...

int main(int argc, char * argv[])
{
    int cp_from = open(argv[1],O_WRONLY);
    int cp_to = open(argv[2],O_WRONLY|O_CREAT,0777);    //make a descriptor for each file
    int size = lseek(cp_from,0,SEEK_END);               //check the size of the first file
    lseek(cp_from,0,SEEK_SET)                           //return to the start of the file, don't know if that's needed
    char *buf = (char*) malloc (size*sizeof(char));     //allocate enough memory to fit all text from 1st file into char array
    read(cp_from,buf,sizeof(buf)-1);                    //read from the 1st file to the char array
    printf("%s",buf);                                   //print the buf
    close(cp_from);
    close(cp_to);
...

So, later I would write() the "buf" to "cp_to" and that would (hopefully) work. But, here's only half of the work because it stopped working at this point, "buf" is empty and I don't know why. Any ideas?

That's my first question on StackOverflom, i'm not used to formatting here yet ^^. I changed the "sizeof(buf)-1" to "size", still doesn't work. — ard_evon, May 26 '16 at 17:12
"still doesn't work" is not useful. Explain the results and what your expectations were. — chux - Reinstate Monica, May 26 '16 at 20:33
the posted code is missing the two statements: `#include ` and `#include ` — user3629249, May 28 '16 at 03:53
when calling `open()`, always check the returned value to assure the operation was successful. — user3629249, May 28 '16 at 03:55
the expression: `sizeof(char)` is defined as 1 in the standard, multiplying anything by 1 has not effect on the parameter passed to `malloc()` and just clutters the code. Suggest removing that expression — user3629249, May 28 '16 at 03:58
when calling `read()`, 1) always check the returned value to assure the operation was successful. 2) the `read()` function does not NUL terminate the input buffer and there is no assurance that the input buffer does not already contain 1 or more NUL bytes. So this line: `printf("%s",buf);` will not work. Suggest using: `fprintf( stdout, "buf, size );` — user3629249, May 28 '16 at 04:02
this line: `lseek(cp_from,0,SEEK_SET) ` is missing a trailing semicolon ';' — user3629249, May 28 '16 at 04:08
the prototype for `malloc()` is in stdlib.h, so the posted code is missing: `#include ` — user3629249, May 28 '16 at 04:10
never access any parameter from main() beyond `arg[0]` with out first checking `argc` to assure that command line parameter was actually entered. — user3629249, May 28 '16 at 04:12
the descriptor: cp_to is set but never used, Overall, the posted code does not perform the desired operation. suggest replacing: printf( "%s"buf); with `write(cp_to, const buf, size);` — user3629249, May 28 '16 at 04:19

score 1 · Answer 1 · edited May 23 '17 at 12:07

1

Here are some review points:

Don't cast the return value of malloc() in C.
Don't use sizeof on a heap pointer thinking it will return anything to do with the allocated buffer's size; it won't. You'll get the size of the pointer.
Use proper types, not just int for everything. Types matter, and not all types are like int.
Don't treat random data read from a file as a string.
Don't do I/O and not check return values. I/O can fail.
... so can memory allocations.

It's probably better to use a small (or smallish) fixed-size buffer, and read/write in a loop. That way your program uses a bounded amount of memory, regardless of the size of the file.

edited May 23 '17 at 12:07

Community

1
1

answered May 26 '16 at 16:57

unwind

391,730
64
469
606

thanks for the tips. Yeah, i thought about using fixed size buffers but i just wanted to see how all those functions would work together (i kinda new to C programming in Linux). I changed the "sizeof(buf)-1" to "size-1" and it still doesn't work. – ard_evon May 26 '16 at 17:15
You can also add "Always use proper types". For example, `lseek` returns `off_t`, not `int`, per http://pubs.opengroup.org/onlinepubs/9699919799/functions/lseek.html – Andrew Henle May 26 '16 at 19:10

score 0 · Answer 2 · answered May 28 '16 at 04:30

the following code:

still has a warning about unused variable argc (see comments)
is missing code to check for error indications returned from system function calls
actually works

and now the code

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

#include <unistd.h>

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

int main(int argc, char * argv[])
{

    int cp_from = open(argv[1], O_RDONLY);

    int cp_to = open(argv[2], O_WRONLY|O_CREAT,0777);

       //make a descriptor for each file


    size_t size = (size_t)lseek(cp_from,0,SEEK_END);               //check the size of the first file
    lseek(cp_from,0,SEEK_SET);
    //return to the start of the file, don't know if that's needed

    char *buf = malloc (size);     //allocate enough memory to fit all text from 1st file into char array
    read( cp_from, buf, size );                    //read from the 1st file to the char array
    write( cp_to, buf, size );                                   //print the buf
    close(cp_from);
    close(cp_to);
}

oh, returning the file pointer to the start of the file is absolutely needed, otherwise the `read()` function would only return a 0, indicating EOF — user3629249, May 28 '16 at 04:35

read() and buffer size errors in (C Linux)

2 Answers2