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?

CodePudding user response：

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.

CodePudding user response：

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.