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

This is a discussion on Is this the right/best way to free up the memory within the C Programming forums, part of the General Programming Boards category; I got my code to work, but need some feedback if I am free up the memory correctly? Is there ...

  1. #1
    Registered User
    Join Date
    Jun 2013
    Posts
    7

    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);
    
    
    }

  2. #2
    ZuK
    ZuK is offline
    Registered User
    Join Date
    Aug 2005
    Location
    Austria
    Posts
    1,990
    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

  3. #3
    Registered User
    Join Date
    May 2010
    Posts
    2,698
    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

  4. #4
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,452
    Additionally,

    > while ((c=fgetc(myfile)) != EOF)
    This loop does not check to see if the buffer is going to overrun.

    > size = strlen(out)+1;
    You need to explicitly append a \0 if you want to use strlen.
    It's largely a waste of effort, you have the length in x already.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  5. #5
    Registered User
    Join Date
    Jun 2013
    Posts
    7
    Quote Originally Posted by jimblumberg View Post
    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.

  6. #6
    Registered User
    Join Date
    Jun 2013
    Posts
    7
    Thanks Salem.

  7. #7
    Registered User
    Join Date
    May 2010
    Posts
    2,698
    Are you now properly terminating your array to make it a C-string?

    You may want to post your modified code.

    Jim

  8. #8
    Registered User
    Join Date
    Jun 2013
    Posts
    7
    Quote Originally Posted by jimblumberg View Post
    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);
    
    
    }

  9. #9
    Registered User
    Join Date
    May 2010
    Posts
    2,698
    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

  10. #10
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,452
    > static char c;
    There is no point in this being static.
    Also, it should be an int, so you can compare it with EOF properly.

    > if(!realloc(out, size))
    1. realloc can move the memory, even if it's making the block smaller. Technically, it could also fail to make it smaller.
    2. As written, you'll chop the \0 off the end of the new block.

    You should do something like
    Code:
    void *temp = realloc( out, size+1 );
    if ( temp != NULL ) {
        out = temp;
    } else {
      // out still points to the oversized block
    }
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  11. #11
    Registered User
    Join Date
    Jun 2013
    Posts
    7
    thanks for your time everyone.!

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 16
    Last Post: 12-28-2012, 03:07 PM
  2. How does free() know how much memory it has to free?
    By shiroaisu in forum C Programming
    Replies: 14
    Last Post: 09-09-2011, 11:28 AM
  3. free RAM memory
    By koyboy in forum C++ Programming
    Replies: 1
    Last Post: 05-11-2008, 07:03 AM
  4. How to free memory ?
    By Brw_Abhi in forum C++ Programming
    Replies: 8
    Last Post: 06-05-2007, 01:19 AM
  5. When to free up memory?
    By thetinman in forum C Programming
    Replies: 7
    Last Post: 09-27-2005, 03:22 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21