Thread: Is this the right/best way to free up the memory

I got my code to work, but need some feedback if I am free up the memory correctly? Is there a way to make sure that I have clear up the memory? I just wanted to get in the good habit since I am pretty new at this. Thanks for the feed back:

Code:

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


char *readfile(const char *filename);


int main(int argc, char *argv[]) {
  char *pms;
  int size = 1;
  pms = readfile(argv[1]);
  printf("%s", pms++);
  return (0);
}


char *readfile(const char *filename){
  static char c;
  char *out;
  FILE *myfile;
  int size;
  out = (char *)malloc(24130);
  int x=0;
  myfile = fopen(filename, "r");
  while ((c=fgetc(myfile)) != EOF){
        out[x]=c;
        x++; }
  //resize the memory here
  size = strlen(out)+1;
  if(!realloc(out, size))
  {
     printf("Unable to allocate memory");
     return (0);
  }
  return(out);
  free(out);


}

You're not freeing the memory at all. The free call comes after the return statement so it's never executed.
You are using the returned memory in main so it has to be freed after use in main().
Also your logic to realloc the memory looks a little bit strange. It only works if realloc alwais reduces the size.
Kurt

but need some feedback if I am free up the memory correctly?

No your not freeing your memory correctly. When the program encounters a return statement it immediately returns to the calling function, so your free() call is never reached.

Also if your file contains more than 24130 characters you will access your array out of bounds. Plus since you are using fgetc() and you don't nul terminate the array you can't use any of the C-string functions like strlen() or printf("%s").

Also why the increment in the printf() in main()?


Jim

Originally Posted by jimblumberg

No your not freeing your memory correctly. When the program encounters a return statement it immediately returns to the calling function, so your free() call is never reached.

Also if your file contains more than 24130 characters you will access your array out of bounds. Plus since you are using fgetc() and you don't nul terminate the array you can't use any of the C-string functions like strlen() or printf("%s").

Also why the increment in the printf() in main()?


Jim

Jim,

Thanks. Yeah.. I read both your and Kurt's feedback. I moved my free function to main right before return. I also took off the increment in the printf. I guessed I thought walk through each char, but this is great. Thanks for reviewing. I have to come up with a better way on allocating the memory.

Thanks Salem.

Are you now properly terminating your array to make it a C-string?

You may want to post your modified code.

Jim

Originally Posted by jimblumberg

Are you now properly terminating your array to make it a C-string?

You may want to post your modified code.

Jim

Here is the revised code. Still not sure about the way I allocated the memory. Just could not figure out without having to put in the initial fix size first.

I don't know the file size until after the loop.

Thanks,

Code:

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


char *readfile(const char *filename);


int main(int argc, char *argv[]) {
  char *pms;
  int size = 1;
  pms = readfile(argv[1]);
  printf("%s", pms);
  //add free memory here ..
  free(pms);
  return (0);
}


char *readfile(const char *filename){
  static char c;
  char *out;
  FILE *myfile;
  int size;
  //still need to work on better way to allocat memory
 // not sure where/how to place this without having to know the actual size.
  out = (char *)malloc(94130);
  int x=0;
  myfile = fopen(filename, "r");
  while ((c=fgetc(myfile)) != EOF){
        out[x]=c;
        x++; }
  //Add Null terminate out
  out[x] ='\0';
  //resize the memory here
  size = strlen(out);
  if(!realloc(out, size))
  {
     printf("Unable to allocate memory");
     return (0);
  }
  return(out);


}

You still need to insure you don't access your array out of bounds during your read by insuring x is less than the size you allocated.


In a C program you don't need to cast the return value from malloc.

Jim

thanks for your time everyone.!