C - realloc() pointer then set value inside function

Question

I'm trying to write a function in C that reads a file into char array passed to it.

void read_file(char* path, char** bufptr, char* buffer){
  /* open the file, and exit on errors */
  FILE *fp = fopen(path, "r");
  if(fp == NULL){
    perror("Failed to open file");
    exit(1);
  }
  /* figure out how long the file is to allocate that memory */
  fseek(fp, 0, SEEK_END);
  long length = ftell(fp);
  rewind(fp);
  /* allocate memory */
  *bufptr = realloc(*bufptr, length+1);
  fread(buffer, length, 1, fp);
  buffer[length] = 0;
}

The idea is that I would use it something like this:

int main(int argc, char** argv){
  char* somestring = malloc(1024);
  core_read_file("/some/file/path", &somestring, somestring);
  printf("In main(): %s\n", somestring);
  free(somestring);
  return 0;
}

However whenever I use it, while the program compiles, the printf prints nothing to the console. I'm starting out and kind of understand the idea of "indirection" to a very very basic degree, but can someone explain to me why my code doesn't work the way I expect it to, and how I should go about implementing this function.

(This isn't a homework assignment so any solution or method that works is perfect)

I don't think you need the buffer argument to read_file. Simply write *bufptr anywhere you're currently writing buffer, e.g. fread(*bufptr, ...); (*bufptr)[length] = 0 — user4815162342
– user4815162342, Commented Oct 2, 2017 at 11:41
You should open the file with rb mode. Also the usual "you're overwriting the old pointer in realloc" complaint... — Antti Haapala
– Antti Haapala, Commented Oct 2, 2017 at 11:46
Instead of realloc, could you malloc the right amount initially and return that? — Ry-
– Ry- ♦, Commented Oct 2, 2017 at 11:47
If the realloc moves the data to a new address, then buffer points to an invalid address (memory that is no longer allocated). — Klas Lindbäck
– Klas Lindbäck, Commented Oct 2, 2017 at 11:48
you have also check return value of functions you are calling (like malloc, realoc, fread) and your core_read_file function instead of void - make to return number of bytes read of error if something goes wrong — Artemy Vysotsky
– Artemy Vysotsky, Commented Oct 2, 2017 at 11:54

Jabberwocky · Accepted Answer · 2017-10-02 12:24:57Z

The last two lines of read_file should be:

  fread(*bufptr, length, 1, fp);
  (*bufptr)[length] = 0;

The pointer in the newly allocated buffer is in *bufptr, not in buffer.

But your program is overly complicated, you don't need to pass three parameters do read_file. Two is enough, like this:

void read_file(char* path, char** bufptr) {
  /* open the file, and exit on errors */
  FILE *fp = fopen(path, "r");
  if (fp == NULL) {
    perror("Failed to open file");
    exit(1);
  }
  /* figure out how long the file is to allocate that memory */
  fseek(fp, 0, SEEK_END);
  long length = ftell(fp);
  rewind(fp);
  /* allocate memory */
  *bufptr = realloc(*bufptr, length + 1);
  fread(*bufptr, length, 1, fp);
  (*bufptr)[length] = 0;
}    

int main(int argc, char** argv) {
  char* somestring = malloc(1024);  //  char* somestring = NULL would be even better here
  read_file("readme.txt", &somestring);
  printf("In main(): %s\n", somestring);
  free(somestring);
  return 0;
}

There is still no error checking for realloc here for brevity.

Artemy Vysotsky · Accepted Answer · 2017-10-02 12:44:20Z

If you are going to allocate memory inside the function - there is no need to allocated outside. But there should be documented aggreement - that in case of failure - nothing is left unfreed, in case of success - calling function is responsible to free the buffer pointer.

Your read function has to have a lot more error checking and also has to close the file on any branch.

Here is the sample code that does the reading:

#define _CRT_SECURE_NO_WARNINGS

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

long read_file(const char* path, char** bufptr) {
    char* buffer;
    int res;
    size_t read;
    FILE *fp;
    if (path == NULL || bufptr == NULL) {
        perror("Invalid parameters");
        return -6;
    }
    /* open the file, and exit on errors */
    fp = fopen(path, "rb");
    if (fp == NULL) {
        perror("Failed to open file");
        return -1;
    }
    /* figure out how long the file is to allocate that memory */
    res = fseek(fp, 0, SEEK_END);
    if (res != 0) {
        perror("Failed to seek file");
        fclose(fp);
        return -2;
    }
    long length = ftell(fp);
    if (length <= 0) {
        perror("Failed ftell");
        fclose(fp);
        return -3;
    }
    rewind(fp);
    /* allocate memory */
    buffer = malloc(length + 1);
    if (buffer == NULL) {
        perror("Out of memory");
        fclose(fp);
        return -4;
    }
    read = fread(buffer, 1, length, fp);
    fclose(fp);

    if ((long)read != length) {
        perror("Failed to read whole file");
        free(buffer);
        return -5;
    }
    buffer[length] = 0;
    *bufptr = buffer;
    return length;
}
int main() {
    char* somestring;
    long res = read_file("c:/key.txt", &somestring);
    if (res < 0) {
        //nothing is leaked - everything opened or allocated was closed and freed by the function
        exit(res);
    }
    printf("In main(): %s\n", somestring);
    free(somestring);
    return 0;
}

Collectives™ on Stack Overflow

C - realloc() pointer then set value inside function

2 Answers 2

Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Related