C Programming- Malloc/Free

Question

So I have a code and when I run it, it hangs when I enter a size greater than 3. When it's exactly 3 it runs smoothly. I narrowed down the problem to malloc and free and I don't know what the problem is. I'm new at this so any help is appreciated.

do  //repeatedly ask the user to put a number between 3-9
{ 
 printf("Enter the size of the game board between 3-9: ");
 scanf("%d", &size);
}while(size<3 || size>9);

if((board = (char***)malloc(sizeof(char**)*size))==NULL)
  printf("Memory Allocation failed\n");
  for(i=0; i<size; i++)
  {
    if((board[i] = (char**)malloc(sizeof(char*)*size))==NULL)
     printf("Memory Allocation failed\n");
     for(j=0; j<size; j++)
     {
       if((board[i][j] = (char *)malloc(sizeof(char)*4))==NULL)  
         printf("Memory Allocation failed\n");
         strcpy(board[i][j], "Go");
     }   
 } 
/*************Some random code ***********/ 

free(board);
for(i=0;i<size;i++)
{
 free(board[i]);
 for(j=0;j<size;j++)
   free(board[i][j]);
}

The order of `free()` should be in exact reverse order of `malloc()`. — Alok Save, Apr 09 '12 at 02:38
When you use 'free(board[i]);' you actually lose the pointer to the memory you have allocated so you can't use 'free(board[i][j]);'. Use free(board[i]); after the second for. — Theocharis K., Apr 09 '12 at 02:41
@Als: It doesn't have to be in the *exact* reverse order. If you have an array of 3 pointers, you can `malloc` them in the order 0, 1, 2, then `free` them in the same order. You just can't `free` a pointer before `free`ing what it points to. — Keith Thompson, Apr 09 '12 at 04:18
@KeithThompson: True, It doesn't have to, but I consider it a good practice rather advice especially with multidimensional arrays for new users.Though it abstracts the purpose and the real reason which you quoted, it works flawlessly abstracting the details. — Alok Save, Apr 09 '12 at 04:21

score 2 · Answer 1 · answered Apr 09 '12 at 03:01

2

The problem is you access board after you freed it. You should release memory in exactly the reverse order that you malloc it.

An alternative approach is that you can allocate all the memory you need in a whole, like

 char ***board = NULL;
 char  **rows  = NULL;
 char   *data  = NULL;

 if((board = (char***)malloc(sizeof(char**)*size))==NULL)
  printf("Memory Allocation failed\n");
 if((rows = (char**)malloc(sizeof(char*)*size*size))==NULL)
     printf("Memory Allocation failed\n");
 if((data = (char *)malloc(sizeof(char)*size*size*4))==NULL)  
     printf("Memory Allocation failed\n");

 for (i = 0; i < size; i++) {
     int board_offset = i * size;
     board[i] = rows[board_offset];
     for (j = 0; j < size; j++) {
         int row_offset = board_offset + j;
         rows[row_offset] = data[row_offset * 4]；
         stcpy(data[row_offset * 4], "GO");
     }
 }

 free(board);
 free(rows);
 free(data);

answered Apr 09 '12 at 03:01

Summer_More_More_Tea

12,740
12
51
83

hey just have a question. Is it common to check for malloc failure? It almost never happens (unless you corrupt memory or actually run out of it) and I believe it will be adding a considerable expense to if() check every malloc. – Lefteris Apr 09 '12 at 03:19
@Lefteris Agree, and I seldom check it to be frank. I just follow the style from original post:). However, it is necessary for robust code I think. – Summer_More_More_Tea Apr 09 '12 at 03:44
Oh I did not notice that he did so in the OP too. Nice to see you agree. Well it depends on what you mean by robust code. I mean if I was to if() check every malloc I guess using another language would be better :P – Lefteris Apr 09 '12 at 03:47
@Lefteris: Not necessarily, you could simply wrap `malloc` in a function that exits on error, you'll still only be calling one function to allocate memory, it'll just have a different name. You shouldn't continue the way the OP does if `malloc` fails. – AusCBloke Apr 09 '12 at 03:57
@AusCBloke Yes ofcourse you can wrap it in a function or even make a neat macro out of it but this is the point I am trying to make. You are adding additional function calling overhead to every malloc call. It all depends on where you wanna set the tradeoff between efficiency and safer programming I suppose. I might have been checking malloc if I had gotten more failed malloc calls, but in the systems I have worked in malloc only failed if I had made a programming error before and corrupted memory. How would you handle a malloc failure in a real program other than just quit? – Lefteris Apr 09 '12 at 04:07
@Lefteris: You should check `malloc()` for error **always**, It gives you error only on corrupting something is a *myth*, it might give you error for any reason,the function doesnt give you an assurance of *when* & *why* it can fail, but it gives you the assurance that *whenever* it fails it will return `NULL`. *How would you handle a malloc failure in a real program other than just quit?* You can have alternate memory management schemes where if `malloc()` fails one might allocate memory through some other alternate mechanism – Alok Save Apr 09 '12 at 04:13
Checking for malloc failure isn't nearly as common as it should be. As @Als says, you should **always** check for `malloc()` failure, even if the response is just to abort the program. – Keith Thompson Apr 09 '12 at 04:20
Additionally, since this wasn't tagged C++, C experts recommend not casting the return value of `malloc()` as it's not necessary in C and can hide an error if you forget to `#include ` I know nobody ever forgets something so simple, but I still do... – hellork Apr 09 '12 at 04:23
@AI hey thanks for the input. I suppose then that if an alternate memory management scheme does not exist then there is no option other than quitting. And if it is for debugging purposes you could have a macro wrap of malloc that does the check which prints out where the failure occured (line and function). That way you can always check malloc when in the development stage of the project but for the shipping code just remove the checks. This is what I currently do. Sorry to be using comments to ask this but what do you think of this approach? – Lefteris Apr 09 '12 at 04:26
@Lefteris: The problem with your approach is that it *assumes*, if `malloc()` doesnt fail on your debug build it won't fail on the release build either.No, its not true.It *may*! So,What happens if does? Your program goes on further and gives a abrupt crash,not a very desirable behavior IMO.You **must** check for `malloc` failure even in release builds, Don't consider it an overhead,it's how `malloc` is meant to be used.If you think of performance, profile your code and find bottlenecks then fix them,ignorying `malloc` error handling for performance is a pre-optimization & IMO a dangerous one. – Alok Save Apr 09 '12 at 04:39
Hmm thanks for your opinion. I can see your point and I appreciate taking the time to detail your logic behind the opinion. I will definitely consider it. Good thing with tge approach in my projects is that it's just a matter of redefining the macro for release builds. – Lefteris Apr 09 '12 at 04:48
@Lefteris, just to give it a call, I consider the check of the return of `malloc` a pre-optimization, especically if it results in such awful code as was given by the OP. Assigning the return of `malloc` inside an `if` expression is really bad style and distracts starters like this one from learning more important things and from structuring their code correctly. – Jens Gustedt Apr 09 '12 at 06:51
@JensGustedt thanks for the feedback Jens I really appreciate it. I believe that having a wrapping macro which basically lets you change how you call malloc depending on your decision is a good approach since you basically have a choice and changing it is just a matter of editing a line of code. It's the best of both worlds I suppose. But having an if() check inside your code in every malloc certainly makes the code look ugly :P – Lefteris Apr 09 '12 at 07:03

C Programming- Malloc/Free

1 Answers1

Linked