OK, I've boiled down what it appears you were attempting to do in your example. One logic issue you are having is in initGraph
. There you want to allocate for a struct Graph
, not a struct Vertices
or struct Edge **adjMat;
. Unless you are adding a value for either, then just set those pointers to NULL
until such a time as you actually need to allocate for either.
First, let's reorder your struct
definitions so any struct required as part of another is defined before it is required, e.g.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <math.h>
struct Vertices {
int id;
char name[15];
float xPos;
float yPos;
};
struct Edge {
int id;
struct Vertices *start;
struct Vertices *end;
};
struct Graph {
int VertexCounter;
struct Vertices *vertex;
struct Edge **adjMat;
};
(note: your instance of MyGraph
has been removed -- it is unnecessary if you are allocating for graph
)
Let's skip forward and look at how your pointer will be handled in main()
, e.g.
Edit Changed Parameter to addVertex
Note in all code below, the parameter for addVertex
was changed from struct Graph **graph
to struct Graph *graph
as there is no reallocation of graphpointer
in addVertex
.
int main (void) {
struct Graph *graphPointer = NULL;
initGraph (&graphPointer);
test(graphPointer);
addVertex (graphPointer);
test(graphPointer);
freeGraph (graphPointer); /* don't forget to free what you allocate */
}
Above, graphpointer
is the only pointer needed. You then pass the address of graphpointer
to initGraph
so that initGraph
can operate on the original and the allocation within initGraph
will be available back in main()
.
To make that happen, you can do the following in initGraph
(note: the allocation for any other members in struct Graph
are delayed until you actually have something to add). If you try and pre-allocate for something that will not occur until some unknown point later in your code -- you create a recipe for disaster. Allocate for what you need, when you need it. (that doesn't prevent you from allocating for a block of each and keeping a counter to reduce the number of realloc
required)
Your initGraph
then becomes:
void initGraph (struct Graph **graph)
{
*graph = calloc (1, sizeof **graph);
if (!*graph) { /* validate EVERY allocation */
perror ("calloc-*graph");
exit (EXIT_FAILURE);
}
(*graph)->vertex = NULL; /* set pointers NULL until you add_vertex */
(*graph)->adjMat = NULL; /* or you add_adjmat */
(*graph)->VertexCounter = 0; /* initialize counter */
}
Above you have initialized one struct Graph
-- nothing more -- as it should be. While with initGraph
you were required to pass the address of graphpointer
so the allocation could be seen back in main()
, with addVertex
all you need to pass is graphpointer
-- because the address of that pointer does NOT change in addVertex
. All you are doing is allocating for a member of struct Graph
in graphpointer
which can simply be assigned to the vertex
pointer. Now when you are ready to add a vertex, you can do:
void addVertex (struct Graph *graph)
{
/* always realloc using a temporary pointer */
void *tmp = realloc (graph->vertex, (graph->VertexCounter + 1) *
sizeof *graph->vertex);
if (!tmp) { /* validate EVERY reallocation */
perror ("realloc-(*graph)->vertex");
exit (EXIT_FAILURE);
}
graph->vertex = tmp;
graph->VertexCounter++;
}
Also note in addVertex
all you are operating on is the struct Vertices
member of graphpointer
-- also as it should be. If you have a vertex to add at this point, consider passing it in as a parameter (or the data in as parameters) so that the vertex data can be assigned/copied in addVertex
to keep all add-vertex code in one place.
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. A quick freeGraph
function to free
what has been allocated thus far could be:
void freeGraph (struct Graph *graph)
{
free (graph->vertex);
free (graph);
}
(when you add your allocation for adjMat
, don't forget to add a corresponding free
in freeGraph
)
Now putting it altogether, you could do:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <math.h>
struct Vertices {
int id;
char name[15];
float xPos;
float yPos;
};
struct Edge {
int id;
struct Vertices *start;
struct Vertices *end;
};
struct Graph {
int VertexCounter;
struct Vertices *vertex;
struct Edge **adjMat;
};
void initGraph (struct Graph **graph)
{
*graph = calloc (1, sizeof **graph);
if (!*graph) { /* validate EVERY allocation */
perror ("calloc-*graph");
exit (EXIT_FAILURE);
}
(*graph)->vertex = NULL; /* set pointers NULL until you add_vertex */
(*graph)->adjMat = NULL; /* or you add_adjmat */
(*graph)->VertexCounter = 0; /* initialize counter */
}
void test (struct Graph *graph)
{
printf("Number of Vertices: %d\n", graph->VertexCounter);
}
void addVertex (struct Graph *graph)
{
/* always realloc using a temporary pointer */
void *tmp = realloc (graph->vertex, (graph->VertexCounter + 1) *
sizeof *graph->vertex);
if (!tmp) { /* validate EVERY reallocation */
perror ("realloc-(*graph)->vertex");
exit (EXIT_FAILURE);
}
graph->vertex = tmp;
graph->VertexCounter++;
}
void freeGraph (struct Graph *graph)
{
free (graph->vertex);
free (graph);
}
int main (void) {
struct Graph *graphPointer = NULL;
initGraph (&graphPointer);
test(graphPointer);
addVertex (graphPointer);
test(graphPointer);
freeGraph (graphPointer); /* don't forget to free what you allocate */
}
/* move to add_adjmat functions:
(*graph)->adjMat = calloc (1, sizeof *(*graph)->adjMat);
if (!(*graph)->adjMat) {
perror ("calloc-(*graph)->adjMat");
exit (EXIT_FAILURE);
}
*(*graph)->adjMat = calloc (1, sizeof **(*graph)->adjMat);
*/
(note: the comments at the bottom to the allocations for adjMat
to whatever function will deal with add_adjmat()
, etc..)
Example Use/Output
$ ./bin/graphalloc
Number of Vertices: 0
Number of Vertices: 1
Memory Use/Error Check
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 --leak-check=full ./bin/graphalloc
==19442== Memcheck, a memory error detector
==19442== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==19442== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==19442== Command: ./bin/graphalloc
==19442==
Number of Vertices: 0
Number of Vertices: 1
==19442==
==19442== HEAP SUMMARY:
==19442== in use at exit: 0 bytes in 0 blocks
==19442== total heap usage: 3 allocs, 3 frees, 1,076 bytes allocated
==19442==
==19442== All heap blocks were freed -- no leaks are possible
==19442==
==19442== For counts of detected and suppressed errors, rerun with: -v
==19442== 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.
This is the approach that will allow you to add what you need when you need. Note to minimize reallocations, you can add a VertexAllocated
member and allocate an initial block of 8, 16, 32, etc..
and then when VertexCounter == VertexAllocated
you can call realloc
then and realloc
2 times VertexAllocated
and keep going.
Look things over and let me know if you have questions.