Pointer to pointer of structs indexing out of bounds(?) when I try to index anything other than zero
I'm trying to edit an array (as a pointer) of structs by filling in the default values with new structs that I initialize. Doing so seems to cause some really bizarre problems. I'm learning how to use structs with pointers, so any help is appreciated.
Snippet from the main function (player just saves the startLoc without changing it)
Location** near;
startLoc = initLocation("Base", 5);
player = initPlayer(startLoc);
near = &(startLoc->near);
*near = initLocation("Zero", 0);
*(near + 1) = initLocation("Two", 0);
printf("Start near: %i\n", startLoc->nearCount);
Entire location.c
#include <stdlib.h>
typedef struct Location {
char isValid;
char* name;
struct Location* near;
int nearCount;
} Location;
Location* initLocation(char* name, int nearCount) {
Location* l = malloc(sizeof(Location));
l->name = name;
l->near = calloc(sizeof(Location) * nearCount, 1);
l->nearCount = nearCount;
l->isValid = 1;
return l;
}
Let's start with a basic discussion about a pointer and a pointer-to-pointer. A pointer is simply a variable that holds the address of something else as its value. When you declare a pointer to something, as you have done with your name or near members within your struct you declare a variable that will hold the address in memory where that type object is stored in memory (e.g. the pointer will point to where that object is stored)
When you declare a pointer-to-pointer to type (e.g. Location **near) you have a pointer that holds the address of another pointer as its value. That can be useful in two ways. (1) it can allow you to pass the address of a pointer as a parameter so that the function is able to operate on the original pointer at that address, or (2) it can allow that single pointer to point to a collection of pointers in memory, e.g.
pointer
| pointers allocated struct
near --> +----+ +-------------------+
| p1 | --> | struct Location 1 |
+----+ +-------------------+
| p2 | --> | struct Location 2 |
+----+ +-------------------+
| p3 | --> | struct Location 3 |
+----+ +-------------------+
| .. | | ... |
(a pointer-to-pointer to type struct Location)
In the second case, why choose a pointer-to-pointer as your type instead of just allocating for a collection of that type? Good question. There are two primary reasons, one would be if what you were allocating for can vary in size. For example:
char**
| pointers allocated strings
words --> +----+ +-----+
| p1 | --> | cat |
+----+ +-----+--------------------------------------+
| p2 | --> | Four score and seven years ago our fathers |
+----+ +-------------+------------------------------+
| p3 | --> | programming |
+----+ +-------------------+
| .. | | ... |
or (2) where you want an allocated collection of an even number of objects (such as changing char** above to int**) that can be addressed using 2D-array indexing (e.g. array[2][7])
Allocating for a collection of pointers and objects adds complexity because you are responsible for maintaining two allocated collections, the pointers, and the objects themselves. You must track and reallocate for both your collection of pointers (and the objects -- if needed) and then free() your collection of objects before freeing your allocated block of pointers.
This can be greatly simplified, if you just need some number of the same type object, such as N - struct Location. That gives you a single allocation, single reallocation and single free for those objects themselves (of course each object can in turn contain allocated objects as well). In your case for near it would be similar to:
pointer
|
near --> +-------------------+
| struct Location 1 |
+-------------------+
| struct Location 2 |
+-------------------+
| struct Location 3 |
+-------------------+
| ... |
(a pointer to type struct Location)
In your case you are dealing with needing nested allocated blocks of struct Location. In that sense, where required, you simply need N - struct Location which will all be of the same size and there isn't a compelling need for 2D array indexing. From that standpoint, looking at what you are trying to do (to the best possible guess), simply allocating for blocks of struct Location rather than handling separate blocks of pointers pointing to individually allocated struct Location would seem to make much more sense.
Implementing A Short-Example
While there is nothing wrong with an initLocation() to set up a single struct Location, you may find it makes more sense to simply write an addLocation() function to add a new struct Location to your collection each time it is called. If you initialize your pointer to the collection NULL back in the caller, you can simply use realloc() to handle your initial allocation and subsequent reallocations.
In the following example, we just create a new struct Location for each name in a list and allocate for 3-near objects. You are free to use addLocation() with the near struct Location in each object just as you have with your initial collection, but that implementation is left to you as it is simply doing the same thing on a nested basis.
Putting an addLocation() function together in a manner that looks like what you are attempting, you could do:
Location *addLocation (Location *l, size_t *nmemb, char *name, int nearCount)
{
/* realloc using temporary pointer adding 1 Location */
void *tmp = realloc (l, (*nmemb + 1) * sizeof *l); /* validate EVERY allocation */
if (!tmp) { /* on failure */
perror ("error: realloc-l");
return NULL; /* original data good, that's why you realloc to a tmp */
}
/* on successful allocation */
l = tmp; /* assign reallocated block to l */
l[*nmemb].isValid = 1; /* assign remaining values and */
l[*nmemb].name = name; /* allocate for near */
l[*nmemb].near = calloc(nearCount, sizeof(Location));
if (!l[*nmemb].near) {
perror ("calloc-l[*nmemb].near");
return NULL;
}
l[*nmemb].nearCount = nearCount; /* set nearCount */
(*nmemb)++; /* increment nmemb */
return l; /* return pointer to allocated block of Location */
}
You could then loop filling each with something similar to:
for (size_t i = 0; i < nmemb;) /* loop adding 1st nmemb names */
if (!(l = addLocation (l, &i, names[i], nearCount)))
break;
(note: i is being updated in addLocation so there is no need for i++ in your loop definition)
A complete example could be written as follows. I have added a print function and a function to delete all allocated memory as well. In the call to addLocation below, you will see names[i%nnames] used instead of names[i] and using the counter modulo the total number of names in my list just ensures that a name from the list is provided, no matter how big i gets.
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
typedef struct Location {
char isValid;
char *name;
struct Location *near;
int nearCount;
} Location;
Location *addLocation (Location *l, size_t *nmemb, char *name, int nearCount)
{
/* realloc using temporary pointer adding 1 Location */
void *tmp = realloc (l, (*nmemb + 1) * sizeof *l); /* validate EVERY allocation */
if (!tmp) { /* on failure */
perror ("error: realloc-l");
return NULL; /* original data good, that's why you realloc to a tmp */
}
/* on successful allocation */
l = tmp; /* assign reallocated block to l */
l[*nmemb].isValid = 1; /* assign remaining values and */
l[*nmemb].name = name; /* allocate for near */
l[*nmemb].near = calloc(nearCount, sizeof(Location));
if (!l[*nmemb].near) {
perror ("calloc-l[*nmemb].near");
return NULL;
}
l[*nmemb].nearCount = nearCount; /* set nearCount */
(*nmemb)++; /* increment nmemb */
return l; /* return pointer to allocated block of Location */
}
void prn_locations (Location *l, size_t nmemb)
{
for (size_t i = 0; i < nmemb; i++)
if (l[i].isValid)
printf ("%-12s nearCount: %d\n", l[i].name, l[i].nearCount);
}
void del_all (Location *l, size_t nmemb)
{
for (size_t i = 0; i < nmemb; i++)
free (l[i].near); /* free each structs allocated near member */
free (l); /* free all struct */
}
int main (int argc, char **argv) {
char *endptr, /* use with strtoul conversion, names below */
*names[] = { "Mary", "Sarah", "Tom", "Jerry", "Clay", "Bruce" };
size_t nmemb = argc > 1 ? strtoul (argv[1], &endptr, 0) : 4,
nnames = sizeof names / sizeof *names;
int nearCount = 3; /* set nearCourt */
Location *l = NULL; /* pointer to allocated object */
if (errno || (nmemb == 0 && endptr == argv[1])) { /* validate converstion */
fputs ("error: nmemb conversion failed.\n", stderr);
return 1;
}
for (size_t i = 0; i < nmemb;) /* loop adding 1st nmemb names */
if (!(l = addLocation (l, &i, names[i%nnames], nearCount)))
break;
prn_locations (l, nmemb);
del_all (l, nmemb);
}
Example Use/Output
$ ./bin/locationalloc
Mary nearCount: 3
Sarah nearCount: 3
Tom nearCount: 3
Jerry nearCount: 3
Or, for example if you wanted to allocate for 10 of them, then:
$ ./bin/locationalloc 10
Mary nearCount: 3
Sarah nearCount: 3
Tom nearCount: 3
Jerry nearCount: 3
Clay nearCount: 3
Bruce nearCount: 3
Mary nearCount: 3
Sarah nearCount: 3
Tom nearCount: 3
Jerry nearCount: 3
Memory Use/Error Check
In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.
It is imperative that you use a memory error checking program to ensure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.
For Linux valgrind is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.
$ valgrind ./bin/locationalloc
==13644== Memcheck, a memory error detector
==13644== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==13644== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==13644== Command: ./bin/locationalloc
==13644==
Mary nearCount: 3
Sarah nearCount: 3
Tom nearCount: 3
Jerry nearCount: 3
==13644==
==13644== HEAP SUMMARY:
==13644== in use at exit: 0 bytes in 0 blocks
==13644== total heap usage: 9 allocs, 9 frees, 1,728 bytes allocated
==13644==
==13644== All heap blocks were freed -- no leaks are possible
==13644==
==13644== For counts of detected and suppressed errors, rerun with: -v
==13644== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Always confirm that you have freed all memory you have allocated and that there are no memory errors.
Let me know if this comports with your intent and whether you have any additional questions.