2

I tried to implement a stack of integers in C by overriding a void pointer using functions. The code works fine for the most part, but gives the error on debugging when I free the allocated memory including the stack object and the integer array. I get this error :- ntdll!RtlRegisterSecureMemoryCacheCallback () from C:\WINDOWS\SYSTEM32\ntdll.dll. I use gcc on windows and vscode.

stack.h :-

#ifndef STACK_H 
#define STACK_H 

void stack_init(void**); 
void stack_push(void**,int); 
void stack_pop(void**); 
int stack_top(void*); 
int stack_empty(void*); 
int stack_size(void*); 
void stack_delete(void**);

#endif 

stack.c :-

#include"stack.h" 
#include<stdlib.h> 

typedef struct 
{
    int*arr; 
    int size; 
    int capacity; 
    int empty; 
}stack; 

void stack_init(void**self) 
{
    if(!*self) 
    {
        *self=malloc(sizeof(stack)); 
        ((stack*)(*self))->size=0;
        ((stack*)(*self))->capacity=1; 
        ((stack*)(*self))->arr=(int*)malloc(sizeof(int)*((stack*)(*self))->capacity); 
        ((stack*)(*self))->empty=1; 
    }
    return;
}

void _realloc(stack**self,int Buffer_size)  
{
    (*self)->capacity=Buffer_size; 
    int*new_array=(int*)malloc(sizeof(int)*Buffer_size); 
    for(int i=0;i<(*self)->size;i++) 
        new_array[i]=(*self)->arr[i]; 
    free((*self)->arr);
    (*self)->arr=new_array;
    return;
}

void stack_push(void**self,int data) 
{
    if(*self) 
    {
        if(((stack*)(*self))->size==((stack*)(*self))->capacity) 
            _realloc((stack**)self,((stack*)(*self))->capacity+((stack*)(*self))->capacity/2);
        if(((stack*)(*self))->empty) 
            ((stack*)(*self))->empty=0; 
        ((stack*)(*self))->arr[((stack*)(*self))->size]=data; 
        ((stack*)(*self))->size++;
    }
    return; 
}

void stack_pop(void**self) 
{
    if(*self) 
    {   
        ((stack*)(*self))->size--;
        if(((stack*)(*self))->size==0) 
            ((stack*)(*self))->empty=1; 
    }
    return; 
}

int stack_top(void*self) 
{
    if(self) 
        return ((stack*)self)->arr[((stack*)self)->size-1];
    exit(0); 
}

int stack_empty(void*self) 
{
    if(self) 
        return ((stack*)self)->empty;  
    exit(0); 
}

int stack_size(void*self) 
{
    if(self) 
        return ((stack*)self)->size; 
    exit(0); 
}

void stack_delete(void**self) 
{
    free((((stack*)(*self))->arr)); // The code works fine when this part is commented
    free(*self); 
    return;
}

main.c :-

#include<stdio.h>
#include<stdlib.h> 
#include"stack.h" 

int main(int argc,char**argv) 
{   
    void*stack=NULL; 
    stack_init(&stack); 
    stack_push(&stack,10);
    stack_push(&stack,20);
    stack_push(&stack,30);
    stack_push(&stack,40);
    printf("%d\n",stack_size(stack));
    while(!stack_empty(stack)) 
    {
        printf("%d ",stack_top(stack)); 
        stack_pop(&stack); 
    } 
    printf("\n");
    stack_delete(&stack); // error
    getc(stdin); 
    return 0; 
}

Update: I tested this code in Visual Studio IDE, and the code crashed with HEAP CORRUPTION DETECTED error, but I am not able to figure out how.

DivijM
  • 185
  • 2
  • 3
  • 10
  • That feels like a crash. Have you used a [*debugger*](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) to figure out where in *your* code it happens? – Some programmer dude Jul 02 '22 at 08:20
  • MS VC outputs `4` `40 30 20 10` and exits cleanly so perhaps there is *undefined behaviour* somewhere. – Weather Vane Jul 02 '22 at 08:23
  • 2
    And why use `void*` and casting all over the place? You can declare the `stack` structure type-alias as an *opaque type* in the header file, and be able to use that for all arguments. Like `typedef struct stack stack;`, and then in the source file `struct stack { ... };` – Some programmer dude Jul 02 '22 at 08:23
  • 1
    Also the only function that needs a pointer to a pointer is the `stack_init` function, all other functions can just take a pointer to `stack` as argument. In fact, `stack_init` could *return* the `stack` pointer, and wouldn't need an argument at all. – Some programmer dude Jul 02 '22 at 08:25
  • @Someprogrammerdude Thanks for the suggestion, but I declared the struct inside the c file to make it as abstract as possible. I would also try out your approach. – DivijM Jul 02 '22 at 10:26

1 Answers1

2

I think the problem is due to the fact that the value of the expression capacity + capacity/2, used by stack_push() when it calls _realloc(), is always equal to 1.

Notice that, since capacity is int and has initial value 1, the value of the expression capacity/2 is 0.

The initial stack capacity never increases, even after calling _realloc(), so function stack_push() accesses areas of memory that have not been properly allocated.

slago
  • 5,025
  • 2
  • 10
  • 23