1

I have 2 pieces code:

gb_Graph = (int **)malloc(sizeof(int*)*gb_nVertices);


if (gb_Graph == NULL)
    return false;


gb_Open = (VERTEX *)malloc(sizeof(VERTEX*)*gb_nVertices);
if (gb_Open == NULL)
    return false;


gb_Close = (VERTEX *)malloc(sizeof(VERTEX*)*gb_nVertices);
if (gb_Close == NULL)
    return false;


for (i = 0; i < gb_nVertices; i++)
{
    gb_Graph[i] = (int*)malloc(sizeof(int)*gb_nVertices);

    if (gb_Graph[i] == NULL)
        return false;

    for (j = 0; j<gb_nVertices; j++)
        fscanf(gb_fInput, "%d", &(gb_Graph[i][j]));
}


for (i = 0 ; i<gb_nVertices; i++)
{
    gb_Open[i].Exist = false;
    gb_Open[i].ParentName = -1;
    gb_Open[i].CostPath = 0;
    fscanf(gb_fInput, "%d", &(gb_Open[i].CostHeuristic));

    gb_Close[i].Exist = false;
    gb_Close[i].ParentName = -1;
    gb_Close[i].CostPath = 0;
    gb_Close[i].CostHeuristic = gb_Open[i].CostHeuristic;
}

gb_Open[gb_nStart].Exist = true;

And

gb_Graph = (int **)malloc(sizeof(int*)*gb_nVertices);
if (gb_Graph == NULL)
    return false;


for (i = 0; i < gb_nVertices; i++)
{
    gb_Graph[i] = (int*)malloc(sizeof(int)*gb_nVertices);

    if (gb_Graph[i] == NULL)
        return false;

    for (j = 0; j<gb_nVertices; j++)
        fscanf(gb_fInput, "%d", &(gb_Graph[i][j]));
}


gb_Open = (VERTEX *)malloc(sizeof(VERTEX*)*gb_nVertices);
if (gb_Open == NULL)
    return false;


gb_Close = (VERTEX *)malloc(sizeof(VERTEX*)*gb_nVertices);
if (gb_Close == NULL)
    return false;


for (i = 0 ; i<gb_nVertices; i++)
{
    gb_Open[i].Exist = false;
    gb_Open[i].ParentName = -1;
    gb_Open[i].CostPath = 0;
    fscanf(gb_fInput, "%d", &(gb_Open[i].CostHeuristic));

    gb_Close[i].Exist = false;
    gb_Close[i].ParentName = -1;
    gb_Close[i].CostPath = 0;
    gb_Close[i].CostHeuristic = gb_Open[i].CostHeuristic;
}

gb_Open[gb_nStart].Exist = true;

In first code, it causes a error. If I put a breakpoint after read value from file into gb_Graph variable in 2 codes, there is no difference. But after that, put a breakpoint in gb_Open[gb_nStart].Exist = true; , in first code, values of gb_Graph are modified.

I think that is order of memory allocation. Right?

With variables:

VERTEX *gb_Open;    
VERTEX *gb_Close;       
int **gb_Graph;         

Please explain me why it wrong? I use VS C++ 2012

WhozCraig
  • 65,258
  • 11
  • 75
  • 141

1 Answers1

5

These are both wrong:

gb_Open = (VERTEX *)malloc(sizeof(VERTEX*)*gb_nVertices);
if (gb_Open == NULL)
    return false;

gb_Close = (VERTEX *)malloc(sizeof(VERTEX*)*gb_nVertices);
if (gb_Close == NULL)
    return false;

You're allocating space of gb_nVertices * sizeof(VERTEX*), which is to say, space for gb_nVertices pointers. You want sizeof(VERTEX), and just to be on the safe side, use a syntax-dereference:

gb_Open = malloc(sizeof(*gb_Open)*gb_nVertices);
if (gb_Open == NULL)
    return false;

gb_Close = malloc(sizeof(*gb_Close)*gb_nVertices);
if (gb_Close == NULL)
    return false;

The same problem exists in the second block of code.

Note also, don't cast the result of malloc() in C.

Community
  • 1
  • 1
WhozCraig
  • 65,258
  • 11
  • 75
  • 141
  • Thank you for answer, i copy-paste from old code and wasn't notice that! But why with declare (VERTEX*)malloc(sizeof(VERTEX*)*gb_nVertices) it still allow me assign value into it? Why it didn't have warning or error in build? – Trịnh Hữu Tâm Oct 20 '13 at 07:32
  • 1
    @TrịnhHữuTâm This is C. You can walk straight into undefined behavior without missing a step. Nothing will stop you from writing to memory you don't own and it may even work (and it may not; it may spawn a chorus line of New York sewer rats singing a broadway show tune). Thus the nature of *undefined behavior*. It did *exactly* what you told it to. You just told it the wrong thing. – WhozCraig Oct 20 '13 at 07:45