Implementing undirected graph with adjacency matrix

Question

I was trying to implement an undirected graph with adjacency matrix.
The type of vertex value is integer. And I use double pointer to represent the adjacency matrix.

The question is right after I inputed all the value of vertices as how I
programmed from the code I posted below, the run-time error
occurred . I then added the comment at the line with the error content
from which the error occurs. I know the cause must be the double pointer
I malloced. But I really have no idea about how to debug it.
Can someone help me out? Thank you!

#include <stdio.h>
#include <stdlib.h>
typedef struct node
{
    int adj;
}verNode;
typedef struct graph
{
    verNode **matrix;
    int* verList;
    int verNum;
    int edgeNum;
}graph;
graph* createGraph(int v)
{
    graph *g=malloc(sizeof(graph));
    if(!g) exit(-1);
    g->matrix=NULL;
    g->verList=malloc(sizeof(verNode)*v);
    if(!g->verList) exit(-1);
    g->verNum=v;
    printf("Enter the value of vertices:\n");
    for(int i=0;i<g->verNum;i++)
    {
        printf("Enter the value of vertex %d:\n",i);
        scanf("%d",g->verList);
    }
    return g;
}
verNode** createMatrix(graph *g)
{
    if(!g) exit(-1);
    g->matrix=malloc(sizeof(int*)*g->verNum*g->verNum);
    if(!g->matrix) exit(-1);
    for(int i=0;i<g->verNum;i++)
    {
        for(int j=0;j<g->verNum;j++)
        {
            (*g->matrix)->adj=0; //error:EXC_BAD_ACCESS (code=1,           //address=0x0)
        }
    }
    return g->matrix;
}
void addEdge(graph *g,int v)
{
    if(!g||!g->matrix||!g->verList) exit(-1);
    int ver1,ver2;
    g->edgeNum=v;
    printf("Enter the indexes of the vertices:\n");
    for(int i=0;i<g->edgeNum;i++)
    {
        printf("Enter the index of vertex 1:\n");
        scanf("%d",&ver1);
        printf("Enter the index of vertex 2:\n");
        scanf("%d",&ver2);
        if(ver1>g->verNum-1||ver2>g->verNum-1) exit(-1);
        g->matrix[ver1][ver2].adj=1;
        g->matrix[ver2][ver1].adj=1;
    }
}
void printMatrix(graph *g)
{
    if(!g||!g->matrix||!g->verList) exit(-1);
    printf("Print the adjacency matrix:");
    for(int i=0;i<g->verNum;i++)
    {
        for(int j=0;j<g->verNum;j++)
        {
            printf("%d ",g->matrix[i][j].adj);
        }
        printf("\n");
    }
}
int main() {
    graph *g=createGraph(5);
    verNode **matrix =createMatrix(g);
    g->matrix=matrix;
    addEdge(g,7);

    return 0;
}

score 0 · Accepted Answer · answered Apr 24 '19 at 16:29

Here:

g->matrix = malloc(sizeof(int*) * g->verNum * g->verNum);

you allocate a flat, one-dimensional array of size V×V. You access that array as if it were a two-domensional array: g->matrix[i][j].adj.

Representing the adjacency matrix as two-dimensional array is a good idea, but your allocation must then also be two-dimensional: Allocate an array of V pointers to int, then make each of that pointers a handle to an array of V ints:

g->matrix = malloc(g->verNum * sizeof(*g->matrix));

for (int i = 0; i < g->verNum; i++) {
    g->matrix[i] = calloc(g->verNum, sizeof(*g->matrix[i]));    
}

Things to note:

calloc(n, size) is like malloc(n*size), but it zeroes out the memory first, so that you start with an unconnected graph.
p = malloc(n * sizeof(*p)) is a useful idiom that infers the type from p rather than specifying the type explicitly.
There are other ways to get a two-dimensional array. For example, you could allocate a flat array of dimension V×V as you did and then make matrix an array of pointers into that array, so that matrix[i] represents a row in the flat array. I think for learning purposes, the approach above is better, because it is more straightforward.
Of course you must free the allocated memory later. Do this in the reverse order: Free the matrix[i] first, then the matrix proper.

score 0 · Answer 2 · answered Apr 24 '19 at 16:33

You have declared graph.matrix as a vernode **. You have allocated memory for the particular instance g->matrix, apparently successfully. *g->matrix designates a vernode * appearing at the beginning of that space (and given that you intend to use the space as an array, it would be more conventional to designate that element as g->matrix[0], which is equivalent). But you never assigned a value to that object. Its value is indeterminate, and you invoke undefined behavior by attempting to access it.

Overall, it looks like you've munged together aspects of multiple different approaches. This allocation ...

g->matrix=malloc(sizeof(int*)*g->verNum*g->verNum);

... is wrong in the first place because it is not safe to assume that sizeof(int *) is the same as sizeof(vernode *), and the latter is what you actually need given the declared type of g->matrix. It seems to be wrong also because you are allocating much more space than I would expect for a double-pointer-based approach. One would normally allocate one pointer for each row, not one per element, and then separately allocate memory for the elements of each row. This would then support double indexing as if you had a bona fide 2D array: g->matrix[i][j].adj = 0;.

You can use just a single allocation for all the vertices, but then you would want a single pointer (vernode *matrix;), and you would need to use different index calculations: g->matrix[i * g->verNum + j].adj = 0;.

"_it is not safe to assume that `sizeof(int *)` is the same as `sizeof(vernode *)`_" ? the size of a pointer is always the same and does not depend on the pointed type, fortunately — bruno, Apr 24 '19 at 16:56
That is often the case, @bruno, systematically so in many implementations, but in fact it is *not* guaranteed. — John Bollinger, Apr 24 '19 at 17:23

Implementing undirected graph with adjacency matrix

2 Answers2