I have implemented a pseudo-generic dynamic array in C, for primitive types. It takes enum constants as identifier for each type- INT
, LONG_INT
, DECIMAL
, CHAR
. The main struct list
has a member _assets
, which is a void
pointer (to have a level of abstraction). This void
pointer is type-casted to another struct (which is the main internal working of the container) in the C file implementation. I tested the list with all the types, and it worked perfectly fine. The problem arose when I created 2 lists, performed some operations, and then deleted the both lists together in the same order they were created. It gives me an error:- Critical error detected c0000374
; and says, unable to open 'free-base.cpp'
. The deletion takes play through the free_assets
method. The 2 lists work perfectly fine when I delete the first list object before using the second list object, which is quiet unusual, since they are 2 different objects.
primitive_list.h
#ifndef PRIMITIVE_LIST_H
#define PRIMITIVE_LIST_H
typedef enum { INT, LONG_INT, DECIMAL, CHAR } list_types;
typedef struct
{
void *_assets;
} list;
list *create_list(list_types type);
void push(list **self, ...);
void pop(list **self);
void*at(list *self, const int index);
int empty(list *self);
unsigned length(list *self);
void print_list(list *self);
void free_assets(list **self);
#endif
primitive_list.c
#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>
#include "primitive_list.h"
typedef struct
{
int capacity;
int size;
void *arr;
list_types type;
int empty;
} _list;
_list _create_list(list_types type)
{
_list res;
res.type = type;
res.capacity = 1;
res.size = 0;
res.empty = 1;
return res;
}
void _alloc(_list *self)
{
switch ((self)->type)
{
int n = (self)->capacity;
case INT:
(self)->arr = calloc(n, sizeof(int));
break;
case LONG_INT:
(self)->arr = calloc(n, sizeof(long long));
break;
case DECIMAL:
(self)->arr = calloc(n, sizeof(double));
break;
case CHAR:
(self)->arr = calloc(n, sizeof(char));
break;
}
return;
}
void _realloc(_list *self, size_t Buffer_size)
{
(self)->capacity = Buffer_size;
list_types type = (self)->type;
int n = (self)->capacity;
int s = (self)->size;
if (type == INT) {
int *new_array = (int *)calloc(n, sizeof(int));
for (size_t i = 0; i < s; i )
{
new_array[i] = ((int *)(self)->arr)[i];
}
free((self)->arr);
(self)->arr = (void *)new_array;
} else if (type == LONG_INT) {
long long *new_array = (long long *)calloc(n, sizeof(long long));
for (size_t i = 0; i < s; i )
{
new_array[i] = ((long long *)(self)->arr)[i];
}
free((self)->arr);
(self)->arr = (void *)new_array;
} else if (type == DECIMAL) {
double *new_array = (double *)calloc(n, sizeof(double));
for (size_t i = 0; i < s; i )
{
new_array[i] = ((double *)(self)->arr)[i];
}
free((self)->arr);
(self)->arr = (void *)new_array;
} else if (type == CHAR) {
char *new_array = (char *)calloc(n, sizeof(char));
for (size_t i = 0; i < s; i )
{
new_array[i] = ((char *)(self)->arr)[i];
}
free((self)->arr);
(self)->arr = (void *)new_array;
}
return;
}
void _push(_list *self, ...)
{
if ((self)->empty)
{
(self)->empty = 0;
_alloc(self);
}
if ((self)->size == (self)->capacity)
_realloc(self, (self)->capacity * 2);
va_list arg;
va_start(arg, self);
switch ((self)->type)
{
case INT:
((int *)(self)->arr)[(self)->size] = va_arg(arg, int);
break;
case LONG_INT:
((long long *)(self)->arr)[(self)->size] = va_arg(arg, long long);
break;
case DECIMAL:
((double *)(self)->arr)[(self)->size] = va_arg(arg, double);
break;
case CHAR:
((char *)(self)->arr)[(self)->size] =(char)va_arg(arg, int);
break;
}
(self)->size ;
va_end(arg);
return;
}
void _pop(_list *self)
{
if ((self)->empty)
{
fprintf(stderr,"List is empty!\n");
return;
}
(self)->size--;
return;
}
void *_at(_list *self, const int index)
{
void *res;
switch ((self)->type)
{
case INT:
res = malloc(sizeof(int));
*((int *)res) = ((int *)(self)->arr)[index];
break;
case LONG_INT:
res = malloc(sizeof(long long));
*((long long *)res) = ((long long *)(self)->arr)[index];
break;
case DECIMAL:
res = malloc(sizeof(double));
*((double *)res) = ((double *)(self)->arr)[index];
break;
case CHAR:
res = malloc(sizeof(char));
*((char *)res) = ((char *)(self)->arr)[index];
break;
}
return res;
}
int _empty(_list *self)
{
return self->empty;
}
unsigned _length(_list *self)
{
return self->size;
}
void _print_list(_list *self)
{
for (size_t i = 0; i < self->size; i )
{
switch(self->type)
{
case INT:
printf("%d ", ((int *)self->arr)[i]);
break;
case LONG_INT:
printf("%lld ", ((long long *)self->arr)[i]);
break;
case DECIMAL:
printf("%lf ", ((double *)self->arr)[i]);
break;
case CHAR:
printf("%c ",((char*)self->arr)[i]);
break;
}
}
printf("\n");
return;
}
void _free_list(_list *self)
{
free((self)->arr);
}
list *create_list(list_types type)
{
static list res;
_list obj = _create_list(type);
res._assets = malloc(sizeof(_list));
*((_list*)res._assets) = obj;
return &res;
}
void push(list **self, ...)
{
va_list arg;
va_start(arg, self);
switch (((_list *)(*self)->_assets)->type)
{
case INT:
_push(((_list *)(*self)->_assets), va_arg(arg, int));
break;
case LONG_INT:
_push(((_list *)(*self)->_assets), va_arg(arg, long long));
break;
case DECIMAL:
_push(((_list *)(*self)->_assets), va_arg(arg, double));
break;
case CHAR:
_push(((_list *)(*self)->_assets), (char)va_arg(arg, int));
break;
}
va_end(arg);
return;
}
void pop(list **self)
{
_pop(((_list *)(*self)->_assets));
return;
}
void *at(list *self, const int index)
{
return _at((_list *)self->_assets, index);
}
int empty(list *self)
{
return _empty((_list *)self->_assets);
}
unsigned length(list *self)
{
return _length((_list *)self->_assets);
}
void print_list(list *self)
{
_print_list((_list *)self->_assets);
return;
}
void free_assets(list **self)
{
_free_list(((_list *)(*self)->_assets));
free((*self)->_assets);
}
test.c
#include <stdio.h>
#include <stdlib.h>
#include "primitive_list.h"
int main(int argc, char **argv)
{
//Decimal List
list *nums = create_list(DECIMAL);
push(&nums, 3.14159);
push(&nums, 6.25);
push(&nums, 22.2222);
push(&nums, 100.0);
print_list(nums);
//Character list
list *chars = create_list(CHAR);
push(&chars, 'A');
push(&chars, 'w');
push(&chars, 'Z');
push(&chars, 'q');
push(&chars, 'P');
print_list(chars);
//Code causing the error
free_assets(&nums);
free_assets(&chars);
return 0;
}
CodePudding user response:
Your code here is a bit confusing, as it appears you're burying your underlying structure in another structure for no reason. There's no reason you cannot simply (if you want to keep implementation details private) write typedef struct list list;
in the header and in your implementation have
struct list {
int capacity;
int size;
void *arr;
list_types type;
int empty;
};
This way, there would be much less confusion about having to deal with the _assets
pointer; you could simply address the list members directly without a dereference and cast (this is needlessly confusing).
Your main issue here is actually due to bad memory management in your create_list
function due to its implementation. Specifically, these two lines:
list *create_list(list_types type)
{
static list res;
/* ... */
return &res;
}
What the static
keyword does here is to define a global variable shared between all invocations of this function. That is, every time you call create_list
, the res
variable is set to the same value as it was at the last return
from the function. This is why you can return &res
from this function to get a valid pointer outside the function: the res
variable is stored in global memory for your program instead of in the function's stack frame.
There are then (at least!) two issues with this program:
- The pointer to every
list
you create will be the same, so everylist
created will override the memory of the lastlist
, which not only leaks memory (the pointer to the_assets
structure), but makes only the last created list valid. In fact, yournums
andchars
lists should have exactly the same value oneschars
is created. - You're calling
free
on memory you didn't allocate withmalloc
. Because you return the address of a bona-fide global variable from yourcreate_list
function, you're returning a pointer to memory created by the Operating System when your program is first loaded. You DO NOT own this memory, and attempting to callfree
on it has exactly the effect you're seeing here: it will crash your program.
Both of these issues can be resolved in the same way: properly manage your memory. Generally, if you want to create a structure in memory in C, you should be calling malloc
with the proper size, initializing the value of the new memory, and returning the pointer to it back to the caller. This can be implemented in your case as:
list *create_list(list_types type)
{
list res = malloc(sizeof(list));
_list obj=_create_list(type);
res._assets=malloc(sizeof(_list));
*((_list *)res._assets)=obj;
return res;
}
Or, if you change the layout of list
as I suggest above, simply:
list *create_list(list_types type)
{
list res = malloc(sizeof(list);
res->type = type;
res->capacity = 1;
// ...
return res;
}
CodePudding user response:
You have a problem here:
list*create_list(list_types type)
{
static list res;
_list obj=_create_list(type);
res._assets=malloc(sizeof(_list));
*((_list*)res._assets)=obj;
return &res;
}
This function is called twice in main()
, but returns a pointer to a static variable, meaning it will return the same address every time it is called. So when main()
appears to create pointers to two different lists, they are actually pointers to a single list, namely static list res
.
CodePudding user response:
There are multiple problems in your code:
hiding the implementation from the user does not require an indirect as posted, you can just declare the
struct list
without a definition.create_list
returns the address of astatic
object, no surprise freeing this has undefined behavior, not to mention the fact that all list objects objects are actually the same object.the line
int n = (self)->capacity;
at the beginning of theswitch
block in_alloc()
is never executed. Move this line before theswitch
statement.you should not use identifiers starting with a
_
.parenthesizing
(self)
is useless and confusing.
Here is a simplified version:
primitive_list.h:
#ifndef PRIMITIVE_LIST_H
#define PRIMITIVE_LIST_H
typedef enum { INT, LONG_INT, DECIMAL, CHAR } list_type;
typedef struct list list;
list *list_create(list_type type);
int list_push(list *self, ...);
void list_pop(list *self);
void *list_at(const list *self, int index);
int list_empty(const list *self);
int list_length(const list *self);
void list_print(const list *self);
void list_free(list **self);
#endif
primitive_list.c:
#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>
#include <string.h>
#include "primitive_list.h"
struct list {
list_type type;
int capacity;
int length;
void *arr;
};
list *list_create(list_type type) {
list *res = malloc(sizeof(*res));
if (res) {
res->type = type;
res->capacity = 0;
res->length = 0;
res->arr = NULL;
} else {
fprintf(stderr, "list_create: out of memory!\n");
}
return res;
}
static int list_element_size(list_type type) {
switch (type) {
case INT: return sizeof(int);
case LONG_INT: return sizeof(long long);
case DECIMAL: return sizeof(double);
case CHAR: return sizeof(char);
}
return 0;
}
int list_push(list *self, ...) {
va_list arg;
if (self->length == self->capacity) {
if (self->capacity == 0) {
self->capacity = 1;
self->arr = calloc(self->capacity, list_element_size(self->type));
if (self->arr == NULL) {
fprintf(stderr, "list_push: out of memory!\n");
return 0;
}
} else {
size_t size = self->capacity * list_element_size(self->type);
void *new_arr = realloc(self->arr, size * 2);
if (new_arr == NULL) {
fprintf(stderr, "list_push: out of memory!\n");
return 0;
}
memset((char *)new_arr size, 0, size);
self->arr = new_arr;
self->capacity *= 2;
}
}
va_start(arg, self);
switch (self->type) {
case INT:
((int *)self->arr)[self->length ] = va_arg(arg, int);
break;
case LONG_INT:
((long long *)self->arr)[self->length ] = va_arg(arg, long long);
break;
case DECIMAL:
((double *)self->arr)[self->length ] = va_arg(arg, double);
break;
case CHAR:
((char *)self->arr)[self->length ] = (char)va_arg(arg, int);
break;
}
va_end(arg);
return self->length;
}
void list_pop(list *self) {
if (self->length == 0) {
fprintf(stderr, "list_pop: list is empty!\n");
} else {
self->length--;
}
}
void *list_at(const list *self, int index) {
if (index < 0 || index >= self->length)
return NULL;
void *res = malloc(list_element_size(self->type));
if (res == NULL) {
fprintf(stderr, "list_at: out of memory!\n");
return 0;
}
switch (self->type) {
case INT:
*((int *)res) = ((int *)self->arr)[index];
break;
case LONG_INT:
*((long long *)res) = ((long long *)self->arr)[index];
break;
case DECIMAL:
*((double *)res) = ((double *)self->arr)[index];
break;
case CHAR:
*((char *)res) = ((char *)self->arr)[index];
break;
}
return res;
}
int list_empty(const list *self) {
return self->length == 0;
}
int list_length(const list *self) {
return self->length;
}
void list_print(const list *self) {
for (int i = 0; i < self->length; i ) {
switch (self->type) {
case INT:
printf("%d ", ((int *)self->arr)[i]);
break;
case LONG_INT:
printf("%lld ", ((long long *)self->arr)[i]);
break;
case DECIMAL:
printf("%f ", ((double *)self->arr)[i]);
break;
case CHAR:
printf("%c ", ((char *)self->arr)[i]);
break;
}
}
printf("\n");
}
void list_free(list **self) {
if (*self) {
free((*self)->arr);
free(*self);
*self = NULL;
}
}
test.c:
#include <stdio.h>
#include <stdlib.h>
#include "primitive_list.h"
int main(int argc, char **argv) {
//Decimal List
list *nums = list_create(DECIMAL);
list_push(nums, 3.14159);
list_push(nums, 6.25);
list_push(nums, 22.2222);
list_push(nums, 100.0);
//Character list
list *chars = list_create(CHAR);
list_push(chars, 'A');
list_push(chars, 'w');
list_push(chars, 'Z');
list_push(chars, 'q');
list_push(chars, 'P');
// print the lists
list_print(nums);
list_print(chars);
// free the lists
list_free(&nums);
list_free(&chars);
return 0;
}