realloc fails in visual studio, but work using gcc (ntdll.dll)

Question

I'm using Visual Studio 2017 and I have this code in C. When I run in VS it fails, but when I compile with gcc it runs perfectly. The error is this:

Exception thrown at 0x779BE643 (ntdll.dll) in GestaoFinanceira.exe: 0xC0000005: Access violation reading location 0x00000009.

This is my code.

I'm using this struct type:

struct budget {
    struct tm time;
    float valor;
};

struct geral {
    struct budget *b;
    int nelem_budget;
};

In main I call a function that will read from a file and save the values is the struct g->b[] as the number of elements in g->nelem_budget

int main() {
    struct geral *g = (struct geral *)malloc(sizeof(struct geral));

    readFileBudget(g);
    // just to check if the read was ok
    showBudget(g->b, g->nelem_budget);

    addBudget(g);
    // to check again if the item added is ok
    showBudget(g->b, g->nelem_budget);

    system("Pause");
    return 0;
}

Here is when the error appens:

void addBudget(struct geral *g) {
    int month, year;
    float value;

    // in the future, this values are to be an user input
    month = 5;
    year = 2015;
    value = 5000;

    printf("Month: %d, Year: %d, Value: %f\n", month, year, value);

    g->b[g->nelem_budget].time.tm_mon = month;
    g->b[g->nelem_budget].time.tm_year = year;
    g->b[g->nelem_budget].valor = value;

    g->nelem_budget++;

    struct budget *tmp = NULL;
    tmp = g->b;
    g->b = realloc(g->b, g->nelem_budget * sizeof(struct budget));
    if (g->b == NULL) {          //reallocated pointer ptr1
        free(tmp);
        printf("error-addBudget->realloc");
        exit(EXIT_FAILURE);
    }

    // just for debug
    printf("month: %d, year: %d, value: %f\n", g->b[g->nelem_budget - 1].time.tm_mon, g->b[g->nelem_budget - 1].time.tm_year, g->b[g->nelem_budget - 1].valor);
}

I have an empty value at the end of the g->b array, that's why I first add the values, and then I call the realloc.

`realloc()` is too late. Needs to be before `g->b[g->nelem_budget].time.tm_mon = month;` OR `realloc(g->b, g->nelem_budget * sizeof(struct budget));` is 1 element too small. — chux - Reinstate Monica, Mar 21 '18 at 23:09
Pretty good attempt for a first question. Well done! You need to show where the actual contents of `geral` are initialized. — John3136, Mar 21 '18 at 23:09
Please [edit] your question and show a [mcve] including the input files — Jabberwocky, Mar 21 '18 at 23:20
Have you tried debugging it and see where it fails? This is one of the best methods to find errors in the code. And should be your first step. — Kami Kaze, Mar 21 '18 at 23:35

score 1 · Accepted Answer · answered Mar 21 '18 at 23:21

You need to reallocate first, then add a new element, not the other way round.

I don't like this

struct budget *tmp = NULL;
    tmp = g->b;
    g->b = realloc(g->b, g->nelem_budget * sizeof(struct budget));
    if (g->b == NULL) {          //reallocated pointer ptr1
        free(tmp);
        printf("error-addBudget->realloc");
        exit(EXIT_FAILURE);
    }

for a couple of reasons:

realloc should be called like this:

struct budget *tmp;

tmp = realloc(g->b, (g->nelem_budget + 1) * sizeof *tmp);

if(tmp == NULL)
{
    // error handling
    return 0; // return error code
}

g->b = tmp;
g->nelem_budget++;

Don't do exit in a function other than main. Instead of closing the program, make addBudget return 1 on success and 0 on failure. The caller of addBudget should decide what to do in case of failure, not addBudget. If you for example put these functions in a library, your program would automatically end on error, and that's sometimes something that you don't want to happen.

So, the function should look like this:

int addBudget(struct geral *g) {
    if(g == NULL)
        return 0;

    int month, year;
    float value;

    // in the future, this values are to be an user input
    month = 5;
    year = 2015;
    value = 5000;

    // reallocate first

    struct budget *tmp = NULL;
    tmp = realloc(g->b, (g->nelem_budget + 1) * sizeof *tmp);

    if(tmp == NULL)
    {
        fprintf(stderr, "error-addBudget->realloc\n");
        return 0;
    }

    g->b = tmp;

    // add new budget after reallocation
    printf("Month: %d, Year: %d, Value: %f\n", month, year, value);

    g->b[g->nelem_budget].time.tm_mon = month;
    g->b[g->nelem_budget].time.tm_year = year;
    g->b[g->nelem_budget].valor = value;

    g->nelem_budget++;

    // just for debug
    printf("month: %d, year: %d, value: %f\n", g->b[g->nelem_budget - 1].time.tm_mon, g->b[g->nelem_budget - 1].time.tm_year, g->b[g->nelem_budget - 1].valor);

    return 1;
}

and then in main:

int main() {
    // using calloc to initialize
    // everything with 0
    struct geral *g = calloc(1, sizeof *g);

    if(g == NULL)
    {
        fprintf(stderr, "Not engough memory\n");
        return 1;
    }

    // you should also check of readFileBudget
    // fails
    readFileBudget(g);

    // just to check if the read was ok
    showBudget(g->b, g->nelem_budget);

    if(addBudget(g) == 0)
    {
        free(g->b);
        free(g);
        return 1;
    }

    // to check again if the item added is ok
    showBudget(g->b, g->nelem_budget);


    // don't forget to free the memory
    free(g->b);
    free(g);

    system("Pause");
    return 0;
}

Also see: do not cast malloc

And error messages should be printed to stderr, not stdout.

realloc fails in visual studio, but work using gcc (ntdll.dll)

1 Answers1