Reverse all lines in a file The script below should open a file, read and reverse every line, then overwrite the source file.

#include &lt;stdio.h&gt;
#include &lt;fcntl.h&gt;
#include &lt;unistd.h&gt;
#include &lt;stdlib.h&gt;
#include &lt;string.h&gt;
#include &lt;sys/stat.h&gt;

// reverses the order of characters in every line of SRC file

void validate_cmdline_args(int argc, char **argv) {
  if(argc != 2) {
    printf("Usage: %s SRC\n", argv[0]);
    exit(1);
  }
}

void reverse_string(char *str) {
  char *end = strchr(str, (int)'\0') - 1;
  char tmp;
  while(str &lt; end) {
    tmp = *str;
    *str++ = *end;
    *end-- = tmp;
  }
}

void process_line(char *output_content, char *line) {
  reverse_string(line);
  strcat(output_content, line);
  strcat(output_content, "\n");
}

int main(int argc, char **argv) {
  validate_cmdline_args(argc, argv);

  char* file_content = NULL;
  char* output_content = NULL;
  char *file_content_free_pointer = NULL;

  int file_descriptor;
  if((file_descriptor = open(argv[1], O_RDONLY)) == -1)
    goto errno_error;

  struct stat file_stat;
  if(fstat(file_descriptor, &amp;file_stat) == -1)
    goto errno_error;

  if((file_content = malloc((size_t)file_stat.st_size+1)) == NULL)
    goto errno_error;
  file_content_free_pointer = file_content;

  if(read(file_descriptor, file_content, (size_t)file_stat.st_size) == -1)
    goto errno_error;
  file_content[(int)file_stat.st_size] = '\0';

  if(close(file_descriptor) == -1)
    goto errno_error; 

  if((output_content = malloc((size_t)file_stat.st_size+2)) == NULL)
    goto errno_error;
  output_content[0] = '\0';

  char* delim = "\n";
  char* line = NULL;

  while((line = strsep(&amp;file_content, delim)))
    process_line(output_content, line);

  if((file_descriptor = open(argv[1], O_WRONLY | O_TRUNC)) == -1)
    goto errno_error;

  if(write(file_descriptor, output_content, (size_t)file_stat.st_size) == -1)
    goto errno_error;
  if(close(file_descriptor) == -1)
    goto errno_error; 

  free(file_content_free_pointer);
  free(output_content);

  return 0;
errno_error:
  perror(NULL);
  free(file_content_free_pointer);
  free(output_content);
  return 1;
}


It's solely for fun and practicing Linux system programming, so I've decided not to divide it into smaller functions.

Here's what I find thought-provoking:


instead of reopening a file I'd prefer to open it in such a way that it becomes truncated just before writing,
this constant error-checking is a madness - can you propose a nicer solution?
I find this flow, with almost every single line beginning with if ... != NULL/-1 etc. a bit distracting
I'm not 100% sure if the code is devoid of off-by-one errors (ok, now I'm sure).
Let's get the broad picture right first.


Do you know Schlemiel the Painter? No? Well, read about him on Joel's blog.
Don't follow in his footsteps; they are too deep because he always restarts from the beginning.
You might want to take that opportunity to make your code tolerate zero-bytes.


Now, having gotten that off the agenda, some other gross wastefulness: you are allocating memory on the order of twice the files size. But you could easily avoid that: Only read in a line, transform it, write it to the output.

There you have two basic good choices:


Modify the file in-place, as your transformation does not affect how much space each line needs. You don't even need any additional disk-space.
Write the output to a temporary file, and then atomically update it by moving: rewrite existing file so that it gets replaced by new version atomically, only once fully written


Now, about reducing the tedium of repeating the exact same code. That's one of the (few) places where the C preprocessor shines:

#define TRY(cond) if(cond) {} else goto error;
// Use it
#undef TRY




Now lots of other small things:


Curiously, one normally doesn't extract a functions precondition-checking into another function, as then the precondition is no longer there to see, and anyway the boilerplate is normally longer than what was extracted: main → validate_cmdline_args.
Why do you use strchr to find the end of a string, instead of the better-known (and probably more efficient) strlen?
For completeness, casting the second argument, a character-literal, to int is completely useless because it's already an int.
There's no reason to define tmp in reverse_string outside the loop. Actually, wouldn't that be a good place for a for-loop?

void reverse_string(char* str) {
    for(char* end = str + strlen(str) - 1; str &gt; end; ++str, --end) {
        char tmp = *str;
        *str = *end;
        *end = tmp;
    }
}

Strictly speaking, both your original and my replacement for reverse_string are UB if strlen(str) == 0, because that would create a pointer one before the string. That can be fixed by replacing the first 1 with !!*str or adding an if-condition, but it doesn't matter in practice.
It might make sense to output the filename given to your program on error. That might be useful for anyone trying to understand what went wrong, so perror(argv[1]).