1

I'm having problems figuring out how to free a pointer to an array of integers, my code is as follow:

int (*gameInfo)[2]; // [0] # of fouls || [1] # of players
gameInfo = (int *)malloc(size * sizeof(int*));
for (int i = 0; i < size; i++) {
    memset(gameInfo[i], 0, 2 * sizeof(int));
}

I declare the pointer in the first line, initialize it in the second, and sets all values to 0 inside the for. It works fine.

After I load the new values, and use them I want to free the pointer. I do the following:

for (int i = 0; i < size; i++) {
    free(gameInfo[i]);
}
free(gameInfo);

I'm trying to free the arrays first, and then free the pointer. I get a "has triggered a breakpoint" the first time it tries to execute "free(gameInfo[i])".

I've read that the array, since it doesn't have dynamic allocated memory, doesn't need to be free, but if I remove the for and just leave free(gameInfo); it gives the same error. I've read this in a blog somewhere, so I'm not sure if it's trustworthy.

Thank you!

Bruno Teixeira
  • 103
  • 10
  • 2
    First of all you allocate the wrong amount of bytes for `gameInfo`. Then you forget that you don't allocate memory for `gameInfo[i]` and try to `free` that anyway. – Some programmer dude Apr 25 '18 at 12:14
  • 1
    I also recommend you look into *structure* as what you have would be perfect as an array of *structures* instead of an array of arrays. – Some programmer dude Apr 25 '18 at 12:16
  • Could you explain what I'm doing wrong when allocation the memory? As for the second point, as I said in the last paragraph, if I remove the for (therefore not freeing the arrays) it still gives the same error. Thank you – Bruno Teixeira Apr 25 '18 at 12:16
  • @BrunoTeixeira: `memset` does not allocate memory. You allocated memory for `size` pointers to arrays of 2 `int`s. Now you have a bunch of pointers to pairs of slots for storing pointers to arrays, ok? So, the problem is that these slots point to nothing. – vgru Apr 25 '18 at 12:17
  • @MichaWiedenmann Of course, would appreciate any suggestion. This seemed to be the most simple, but if structure (as pointed by Some programmer dude) is a better option I could definitely do it. – Bruno Teixeira Apr 25 '18 at 12:18
  • @BrunoTeixeira check out here https://stackoverflow.com/questions/8102403/when-to-free-pointer-in-c-and-how-to-know-if-it-is-freed – Anand Choudhary Apr 25 '18 at 12:19
  • @Groo Yes, I've read that, the memory is being allocated in the second line. Not sure if you mean that something is wrong. – Bruno Teixeira Apr 25 '18 at 12:19
  • `gameInfo` is a pointer to (one or more) arrays of ints. You allocated a number of pointers, but haven't allocated actual arrays. The simplest way would be to make it a pointer to structs, this would make it more obvious where the mistake is. As @Someprogrammerdude wrote, `gameInfo` is not an `int*` (it's `int (*gameInfo)[2]` instead), so your `malloc` statement is incorrect too. – vgru Apr 25 '18 at 12:21
  • Do you want to keep two numbers, or two lists of numbers? – Micha Wiedenmann Apr 25 '18 at 12:22
  • 1
    @MichaWiedenmann: neither, he wants a list of pairs of numbers. – vgru Apr 25 '18 at 12:23
  • 3
    A smart tip when using `malloc` and "special" or "hard" types: Use the dereferenced variable in the `sizeof`. Like in your case, `gameInfo = malloc(size * sizeof *gameInfo)` – Some programmer dude Apr 25 '18 at 12:24
  • @Groo I'll try to see if I can solve this with structures then. As for the malloc being incorrect, could you tell me how it should be done? Is it: malloc(size * sizeof(int*[2])) ? – Bruno Teixeira Apr 25 '18 at 12:28
  • @Someprogrammerdude That just did it! Thank you :) I also removed the for, so I wouldn't be free the array like you mentioned. I would like to mark it as the correct answer, if you don't mind to answer the topic with these two changes. Thank you everyone for helping. I'll still look into structures for this :) – Bruno Teixeira Apr 25 '18 at 12:34

3 Answers3

2

First, for your stated purposes, the declaration:

int (*gameInfo)[2]; // [0] # of fouls || [1] # of players

Could be:

int *gameInfo[2]; // [0] # of fouls || [1] # of players

Then, allocation for both elements of the array would look like this:

int size = 10; // for example

gameInfo[0] = malloc(size * sizeof(*gameInfo[0]));
gameInfo[1] = malloc(size * sizeof(*gameInfo[1]));

This essentially creates a quantity of space equivalent to that you would have with a 2D int array such as:

int gameInfo[2][10];

After use, free them:

free(gameInfo[0]);
free(gameInfo[1]);

A better option, as mentioned in the comments might be to create a struct:

typedef struct {
   int fouls;
   int players;
} GAME;

Usage can then include creating, using then freeing an array of games:

GAME *game;
game = calloc(10, sizeof(*game));
...
game[0].fouls = 3;
game[0].players = 2;
...
game[9].fouls = 6;
game[9].players = 3;
...
free(game);
ryyker
  • 22,849
  • 3
  • 43
  • 87
  • 1
    The problem I was having was because of an error in my malloc. However, the structure approach is a excelent idea and I'll implement it. The topic is small and people with the same problem can read the comment and understand what happened, so I'll go and mark your answer as the correct one. Thank you for your help! – Bruno Teixeira Apr 25 '18 at 12:58
  • "This essentially creates space equivalent to what you would have with a 2D int array" No! It allocates two pieces of fragmented memory! Furthermore you can't zero initialize the data with a single call to calloc or memset this way. Just forget about using these pointer look-up tables please. They are complex, slow and unsafe at once. The struct idea is sound and may be the correct solution to the actual problem, but it is no 2D array. – Lundin Apr 25 '18 at 13:01
  • @Lundin - Agreed. no argument. I have clarified that statement in my post. Thanks for pointing out that mistake. However, I have used pointers to multiple instances of struct often, and with performance that satisfies my needs. They enable reading large collections of data, requiring representation using multiple data types, where the quantity of instances is not known until run time. – ryyker Apr 25 '18 at 13:28
0

To begin with, you allocate incorrectly. You are using an array pointer, so it should be set to point at an array, not to some fragmented pointer look-up table.

int (*gameInfo)[players]; // [0] # of fouls || [1] # of players
gameInfo = calloc(1, sizeof( int[fouls][players] ));

...
gameInfo[x][y] = something;
...

free(gameInfo);

That's it.

Not only did this turn the code much more readable, it also improves performance drastically.

Further study: Correctly allocating multi-dimensional arrays.

Lundin
  • 195,001
  • 40
  • 254
  • 396
0
int(*gameInfo)[2];
gameInfo = (int*)malloc(4 * sizeof(int*));
for (int i = 0; i < 2; i++) {
    memset(gameInfo[i], 0, 2 * sizeof(int));
}
free(gameInfo);

After some tests, I updated it. The reason lies on nonalignment of memset and malloc. Obviously you applied 2*sizeof(int) memory and memset 2 times of 2*sizeof(int), which is out of the range of 8 byte you applied. Apply 16 byte, in my system fixed the problem. And thanks for @Lundin for reminding me the problem.

Shore
  • 827
  • 9
  • 24
  • `&gameInfo[0]` -> `gameInfo`. But there are more serious problems here, such as the OP allocating an array of pointers instead of data. – Lundin Apr 25 '18 at 13:04
  • @Lundin wat do you mean by allocating an array of pointers instead of data. The original malloc function allocated a field of 8 byte and send the start address to gameInfo. C allows you to use gameInfo[i] to access the n'th data if you defined the correct pointer type. But what gameInfo itself is a pointer, that contains the address of it......... – Shore Apr 25 '18 at 13:08
  • They are using an array pointer, then allocating an array of pointers, which is something else entirely. And then they don't allocate space for each pointer in the array of pointers either. Which would have been the wrong solution anyway. See my answer for an example of how to do this proper. – Lundin Apr 25 '18 at 13:10