Recreation of cat in C

I have a school project to create a program that works the same as the Linux cat. The code works, but I just need find out where I can Improve it or handle errors. The program has 3 .c files and 1 header file. I'm also not allowed to use system functions; I have to use read(), close(), open() and write() and while loop only.

ft_read.c

#include "ft_read_file.h"

 int    ft_len(char *str)
 {
     int    i;

     i = 0;
     while (str[i] != '\0')
     i++;
     return (i);
  }

  void  ft_error(char *file_name)
  {
       ft_writeln(1, file_name, ft_len(file_name));               
       ft_writeln(1, ": ", 2);
       ft_writeln(1, file_name, ft_len(file_name));
       ft_writeln(1, ":", 1);
       ft_writeln(1, " No such file or directory", 26);
       ft_writeln(1, ".\n", 2);
   }

   int  ft_read(int files, char **file_name)
   {
       int  fd;
       int  i;
       char buf[4096];
       int  size;

       fd = 1;
       i = 1;
       size = 0;
       while (i < files)
       {
           fd = open(file_name[i], O_RDONLY);
           if (fd == -1)
           {
               ft_error(file_name[i]);
                return (1);
            }
             size = read(fd, buf, 4095);
             buf[4095] = '\0';
             ft_writeln(1, buf, size);
             close(fd);
             i++;
          }
          return (0);
     }

main.c

#include "ft_read_file.h"

int main(int argc, char **argv)
{
    if (argc >= 1)
        ft_read(argc, argv);
    return (0);
 }

ft_writeln.c

#include "ft_read_file.h"

void    ft_writeln(int fd, void *buf, int size)
{
       write(fd, buf, size);
}

Use the C standard library

Your code could be greatly simplified if you used the C standard library functions fopen, fclose, fread, fwrite and fprintf instead of the low-level POSIX system calls. The standard libraray also provides strlen which obsoletes your ft_len function but if you use the functions mentioned earlier, you won't have to count the length of a string any more anyway. If these standard library functions are verboten in your assignment, please state this in your question.

Be prepared for read and write to return early

read and write are not guaranteed to read or write all bytes they were asked to. The proper way to use them is therefore to call them in a loop until either all bytes are read / written or an error is encountered.

Be prepared for read and write to fail

read and write may fail in which case they return a negative value and set errno. You should check this and act upon the error appropriately.

Be prepared for files larger than 4095 bytes

You only call read a single time per file. The concerns about read returning early aside, what if the file is larger than your buffer?

Get rid of useless NUL termination

You stick a '\0' byte at the end of your buffer but nothing expects this. You're already passing around an explicit length (which is good, because the buffer may contain embedded NUL bytes). And if you do it anyway, it should be buf[size] = '\0';.

Don't guess the reason for an error

If you cannot open a file, you always report “No such file or directory”. But that isn't the only reason a call to open can fail. You should inspect the value of errno to find out why the operation failed. If you cannot find out the reason, reporting a generic error like “Cannot open file … for reading” is still better than guessing a potentially wrong reason. Reporting a wrong reason is worse than reporting no reason at all because it will mislead your users.

#include all required headers

I don't see any #includes of system headers in your code. Your code depends on the following headers.

• ` for open, read, write and close






• and for the respective flags




In case you have #included them in the header file you didn't show: Don't do that. What functions are needed in the implementation is an implementation detail. You headers should only expose the public API of your module.

Use proper types

The length of an array should be measured in size_t (defined in ) instead of int. The return type of read and write is ssize_t.

Avoid uninitialized variables

It's not the 1980's any more where we had to write code like this.

int i;
// …
i = 1;
// …
while (i < files)
  {
    // …
    ++i;
  }

Instead, we can write this.

for (int i = 1; i < files; ++i)
  {
    // …
  }

The more you limit the scope of a variable, the lesser the risk for accidentally misusing it.

The variables size and fd could be declared only inside the body of the loop. fd could be declared const then.

const int fd = open( … );

It would avoid certain mistakes and you wouldn't be tempted to initialize it with a meaningless value.

Avoid magic numbers

Your choice for a buffer size of 4096 is reasonable but still arbitrary. One day, you might decide to chose a different size. Yet, the constant – or, even worse, constants derived from it, which you cannot search for – is repeated several times in the code. Instead of hard-coding such magic numbers, use a named constant.

#define BUFFER_SIZE 4096

char buffer[BUFFER_SIZE];

const ssize_t count = read(fd, buffer, BUFFER_SIZE - 1);

You could also use sizeof(buffer).

Since C now supports variable-length arrays, you could also use a const variable instead of a macro #define.

Subtracting 1 isn't really needed because NUL termination isn't needed as mentioned above. But when you do need to derive a size, using an expression like this automatically updates the dependent value if the value it depends on changes.

The line

ft_writeln(1, " No such file or directory", 26);

is another example that is waiting for somebody to alter the text and forgetting to update the number. It's not even clear what the 26 means immediately.

In order to further reduce the amount of magic constants, consider using the macros STDOUT_FILENO, STDERR_FILENO and STDIN_FILENO, which are #defined in to 1, 2 and 0 respectively. The benefit is not so much that these constants would change any time soon or people don't know what file descriptor 1 refers to but that in a function call

print(1, "hello, world\n");

the 1` could mean almost anything whereas in

print(STDOUT_FILENO, "hello, world\n");

the meaning of the first argument is much clearer.

Print error messages to standard error output

Programs should output their results to standard output and any logging, warnings or error messages to standard error output. One reason for this is that stan

StackExchange Code Review Q#125720, answer score: 8