0

I have written a function that allocates and initialises a 2D array, like this -

static state **symbols;

void initNFAStates()
{
    int i,j;
    numStates = 256;
    symbols = (state**)malloc(numStates * sizeof(char));
    for(i = 0;i < numStates;i++)
    {
        symbols[i] = (state*)malloc(NUMSYMBOLS * sizeof(state));
        for(j = 0;j < NUMSYMBOLS;j++)
            symbols[i][j] = 0;
    }
}

and a function to print this array, like this -

void printNFAStateTable()
{
    int i, j; 
    for(i = 0;i < numStates;i++)
    {
        printf("%d \t",i);
        for(j = 0;j < NUMSYMBOLS;j++)
            printf("%ld",symbols[i][j]);
        printf("\n");
    }
}

When called consecutively from the main() function, they both work fine. However, the code as follows results in a segfault after reading only the first 32 lines from the array.

int main(int argc, char **argv)
{
    int i;
    clock_t begin, end;
    double timeTaken;
    currNFAState = 0;
    initNFAStates();


    if(getChars(argc,argv) != NULL)
    {
        printNFAStateTable();
        begin = clock();
        regex();
        ...

Similarly, the printf() function causes the same issue, but only when printing a floating point number -

int main(int argc, char **argv)
{
    int i;
    clock_t begin, end;
    double timeTaken;
    currNFAState = 0;
    initNFAStates();
    printf("A floating point number - %f",0.0124f);
    printNFAStateTable();
    ...

I am aware this has to do with the symbols array being static, as the issue does not appear when the array is global. Could anyone explain why this occurs?

PointToPoint
  • 117
  • 1
  • 3
  • 12
  • 2
    `(state**)malloc(numStates * sizeof(char))` is wrong: You're trying to allocate an array of pointers, not an array of chars (also, don't cast malloc). – melpomene Sep 14 '15 at 18:10

2 Answers2

1

Given this declaration:

static state **symbols;

This allocation is incorrect:

symbols = (state**)malloc(numStates * sizeof(char));

The type of *symbols is state *; this is the type of the elements of the array you are dynamically allocating, and I feel confident in asserting that pointers on your machine are larger than char is. This would be a more appropriate allocation:

symbols = malloc(numStates * sizeof(*symbols));

(Note that you do not need to cast the return value of malloc(), and there are good reasons not to do so.)

Having not allocated memory sufficient for all the pointers you want to use, your program exhibits undefined behavior when it tries to access elements at indices that would fall outside the bounds of the allocation. That UB very easily could manifest in the form of library functions modifying memory you did not expect them to modify.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
-2

This is not doing what you expect:

symbols[i][j] = 0;

The reason is that this assumes a singularly allocated block of memory organized as a 2D array. That's not what you've created.

Your code indicates the first dimension is sized at 256, which would look like this:

state symbols[256][NUMSYMBOLS];

If you allocated globally or on the stack. This would be a single block of RAM sized as 256 * NUMSYBOLS * sizeof( state ), where each row is advanced NUMSYMBOLS * sizeof( state ).

What you're doing, however, is create an a array of pointers in one block of RAM, and then allocating additional blocks of RAM for each row. They are unrelated such that access is not going to work using the 2D array syntax.

What you need is first to access the pointer to the row, conceptually:

state *state_row = symbols[ i ];

This gives you the row. Now, get the column;

stat * state_cell = state_row[ j ];

This is expanded to show how to think about it, you can easily choose other specific means of accessing the appropriate cells.

JVene
  • 1,611
  • 10
  • 10
  • 1
    This looks like complete nonsense. – melpomene Sep 14 '15 at 18:15
  • Double-subscript syntax works equally well for a 2D array as for an array-of-arrays. The details of how the program chooses what memory to access are different, but in both cases it works out as one would expect. – John Bollinger Sep 14 '15 at 18:17