Random characters at the end of C read file

Question

I've been trying to read a file using code I got from this answer.

This is my code:

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

char * readfile(FILE * f)
{
    char * buffer = 0;
    long length;
    

    if (f)
    {
        fseek(f, 0, SEEK_END);
        length = ftell (f);
        fseek(f, 0, SEEK_SET);
        buffer = malloc(length);
        if (buffer) fread(buffer, 1, length, f);
        fclose(f);
    }

    return buffer;
}

int main()
{
    FILE * f = fopen("./file.txt", "r");
    printf(readfile(f));
}

My function does return the contents of the file but it also returns random characters.

contents╪▐>c☻ú

And amazingly they seem to change every time I run the code.

contents╗╙ÿ▓.┤

The contents of my file are "contents".

My question is: How can I fix my code for it not to return random characters?

Strings in C need to be NUL terminated. `malloc(length+1)` then after the `fread` NUL terminate to make it a valid C string `buffer[length]='\0'` — kaylum, Dec 19 '21 at 05:07
You should add null termination character to the buffer char array like this `buffer[length]='\0'` — Jayamal Jayamaha, Dec 19 '21 at 05:10
Also, this is a security risk if the file might contain a `%s`. The first argument to `printf` should *always* be a string literal, or at worst the result of calling a `format_arg` function with a string literal argument. — o11c, Dec 19 '21 at 05:38
@kggn, You missed the comment: [fread does not zero-terminate your string. This can lead to some trouble.](https://stackoverflow.com/questions/174531/how-to-read-the-content-of-a-file-to-a-string-in-c/174552#comment40228228_174552). — chux - Reinstate Monica, Dec 19 '21 at 06:33

score 2 · Answer 1 · answered Dec 19 '21 at 05:34

Strings in C need to be null terminated. Your buffer string is not terminated when you read the contents. Essentially you are printing the files contents as desired, but the read just keeps on going in contiguous memory until it reaches a null character by chance.

chux - Reinstate Monica · Answer 2 · 2021-12-19T06:55:44.143

My function does return the contents of the file but it also returns random characters.
How can I fix my code for it not to return random characters?

No. The function returned only a pointer, not any characters.

It is the printf() call that used that pointer, thinking it was a pointer to a string. In C, a string always contains a final '\0', else it is not a string. It was only a pointer to a character array lacking a null character and led to undefined behavior as printf(s) only knows where to start (at s), but not where to end.

Do not print past the end of the allocated data.

A better approach: return a pointer to a string. Allocate enough for the data and a final null character and form a string to let printf() know where to stop.

Best to add error checking too.

// Let calling code close the file
char *readfile_alt(FILE * f) {
  // f invalid? fseek() fail?
  if (f == NULL || fseek(f, 0, SEEK_END)) {
    return NULL;
  }

  long length = ftell(f);
  rewind(f);
  // Did ftell() fail?  Is the length too long?
  if (length == -1 || length >= SIZE_MAX) {
    return NULL;
  }

  // Convert from long to size_t
  size_t ulength = (size_t) length;
  char *buffer = malloc(ulength + 1);
  // Allocation failed? Read incomplete?
  if (buffer == NULL || fread(buffer, 1, ulength, f) != ulength) {
    free(buffer); 
    return NULL;
  }
  buffer[ulength] = '\0'; // Now buffer points to a string

  return buffer;
}

Also, printf() expects a format string where "%" has special meaning. Better to use printf("%s", readfile(f));.

int main() {
  FILE * f = fopen("./file.txt", "r");
  if (f) {
    char *s = readfile(f);
    if (s) {
      printf("%s\n", s);
      free(s); // Free resource when done with them.
    }
    fclose(f); // Free resource when done with them.
  }
}

Even better code would consider the file may unexpectedly contain null characters. Leave that for another day.

Random characters at the end of C read file

2 Answers2