C program to reverse content of a file and write that to another file

Question

I'm having issues with an assignment where I have to take the contents of one file into a buffer, reverse those contents, and write them to another file. This program NEEDS to utilize two functions that look like this:

• int read_file( char* filename, char **buffer );
• int write_file( char* filename, char *buffer, int size);

so far my files look like this:

file_utils.h

 #ifndef UTILS_H
 #define UTILS_H
      int read_file(char* filename, char **buffer);
      int write_file(char* filename, char *buffer, int size);
 #endif

file_utils.c

 #include "file_utils.h"
 #include <stdlib.h>
 #include <stdio.h>
 #include <font1.h>
 #include <string.h>
 #include <sys/stat.h>
 #include <unistd.h>

 int read_file(char* filename, char **buffer) {
      FILE* file1;
      file1 = fopen(filename, "r");

      //gets the size of the file
      struct stat st;
      stat(filename, &st);
      int size = st.st_size;

      buffer = malloc(size);
      read(file1, &buffer, 1);
      return size;
 }

 int write_file(char* filename, char*buffer, int size) {
      FILE* file2;
      file2 = fopen(filename, 'w');

      for (int k = size - 1; k >= 0; k--) {
          char* x = &buffer + k;
          fprintf(file2, "%s", x);
      }
      printf(filename, '\O');
      return 1;
 }

reverse.c

 #include "file_utils.h"
 #include <stdlib.h>
 #include <stdio.h>
 #include <font1.h>
 #include <string.h>
 #include <sys/stat.h>
 #include <unistd.h>

 int main(int argc, char *argv[]) {
      char* buffer;
      char* filename1;
      char* filename2;
      int filesize;

      filename1 = argv[1];
      filename2 = argv[2];

      filesize = read_file(filename1, &buffer);
      write_file(filename2, buffer, filesize);

      return 0;
 }   

and that's all there is. I run it using "clang file_utils.c reverse.c" and I get warnings for file_utils.c like

• incompatible integer to pointer conversion passing 'int" to parameter of type 'const char *' (for the line file1 = fopen(filename, 'r')
• incompatible pointer to integer conversion passing 'FILE *' (aka 'struct_IO_FILE*') to parameter of type 'int' (for the line read(file1, &buffer, 1);)
• the same warning as the first one but for the line file2 = fopen(filename, 'w');
• and incompatible pointer types initializing 'char *' with an expression of type 'char **'; dereferences with * (for the line char* x = &buffer + k;)

on top of all this when I continue on to run the executable as such

./a.out file1 file2

where file 1 has text that should be reversed into file 2, I get a segmentation fault.

Any insight into things I can fix will be much appreciated.

Answer 1

Just off the top of my head, without testing, I see these bugs:

buffer = malloc(size); should be *buffer = malloc(size);

... because buffer is a pointer to pointer to char, you need to dereference it once.

read(file1, &buffer, 1); should be fread(*buffer, 1, size, file1);

... because you opened file1 with fopen, so it's FILE *. read is Unix I/O, not stream I/O, and doesn't use FILE *.

file2 = fopen(filename, 'w'); should be file2 = fopen(filename, "w");

The second argument should be a "string" (pointer to char or array of char). 'w' is a single char.

char* x = &buffer + k; should be char *x = buffer + k;

buffer is a pointer to char, so you want to use it directly, not take its address. Also note the style of putting * next to the variable instead of the type. This is a good habit, because these do not mean the same thing:

char *a, *b, *c;   /* three pointers */
char* a, b, c;     /* one pointer, two chars */

fprintf(file2, "%s", x); should be fprintf(file2, "%c", *x);

The first form treats x as the beginning of a string and will output everything from that point onward until it hits a NUL terminator. You want to output only one char, so use the %c specifier, and dereference x to get a char.

A better way would be fwrite(x, 1, 1, file2);

printf(filename, '\O'); is not needed and doesn't do what you think. It looks like you intended to write a NUL at the end. That would be '\0' (zero), not '\O' (letter O). In any case, it's not needed or wanted. NUL is used to terminate a string in C, not a file. Your output file will be one character longer than it should be if you do this.

Answer 2

The most important issue with your code is here

      char* x = &buffer + k;
      fprintf(file2, "%s", x);

perhaps you mean

      char *x = buffer + k;
      fprintf(file2, "%c", *x);

You also, are mixing IO functions. For a FILE * object you should use fread() instead of read(), there should be an incompatible arguments warning from the compiler.

If there are no warnings (BTW char *x = &buffer + k should trigger another warning), then you should probably enable them explicitly so that your compiler can help you figure out other problems.

Also, check that file1 is not NULL after fopen(), check that fread() did read the requested amount, in general check for every possible error which you can easily infer from the return value of the implied function, if you don't know the meaning of such value then READ THE DOCUMENTATION before using such function.

Answer 3

And finally all together:

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <sys/stat.h>
#include <unistd.h>

int read_file(char* filename, char **buffer) {
  FILE* file1;
  file1 = fopen(filename, "r");

  //gets the size of the file
  struct stat st;
  stat(filename, &st);
  int size = st.st_size;

  *buffer = malloc(size);
  fread(*buffer, size, 1, file1);
  fclose(file1);

  return size;
}

void write_file(char* filename, char*buffer, int size) {
  FILE* file2 = fopen(filename, "w"); int k;

  for (k = size - 1; k >= 0; k--) {
    fwrite(buffer + k, 1, 1, file2);
  }

  fclose(file2);
}

int main(int argc, char *argv[]) {
  char* buffer;
  char* filename1;
  char* filename2;
  int filesize;

  filename1 = "input.txt";
  filename2 = "reverse.txt";

  filesize = read_file(filename1, &buffer);
  write_file(filename2, buffer, filesize);

  free(buffer);

  return 0;
}   

with a live demo. Please add checks for all return values, like malloc() does not return NULL.