Ive tried creating a dynamic database in which the user inputs the size of the database they want to create however im getting a memory leak after a certain amount of size inputs and im unsure what im doing incorrectly as im freeing everything as it should.
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
typedef struct _product_t
{
char *product;
float price;
} product_t;
product_t *newDatabase(product_t *database, int *dbSize, int newSize)
{
product_t *newdatabase = (product_t *)malloc(sizeof(*newdatabase) * newSize);
if (newSize < *dbSize)
{
for (int i = 0; i <= newSize; i )
{
if (database[i].product != NULL)
{
free(database[i].product);
}
}
}
else
{
for (int i = 0; i <= *dbSize; i )
{
if (database[i].product != NULL)
{
free(database[i].product);
}
}
}
for (int i = *dbSize; i < newSize; i )
{
// newdatabase[i].product = (char*)malloc(sizeof(char)*100);
newdatabase[i].product = NULL;
newdatabase[i].price = -1;
}
*dbSize = newSize;
free(database);
return newdatabase;
}
int main(void)
{
product_t *database = NULL;
int dbSize = 0;
char cmd;
do
{
printf("Command?");
scanf(" %c", &cmd);
switch (cmd)
{
case 'q':
printf("Bye!");
break;
case 'n':
printf("Size? ");
int newSize2 = 0;
scanf("%d", &newSize2);
if (newSize2 < 0)
{
printf("Must be larger than 0");
break;
}
else
{
database = newDatabase(database, &dbSize, newSize2);
break;
}
default:
printf("Unkown command '%c'\n", cmd);
}
} while (cmd != 'q');
return 0;
}
#edit number 3 Ive opted on removing the malloc for the product pointer and decided to free all non NULL values with if and else statements
CodePudding user response：
- With the first malloc you allocate newSize pointers to product_t but you want to allocate an array of them instead:
product_t *newdatabase = malloc(newSize * sizeof *newdatabase);
You cannot deference the database pointer after you free it.
Why do you only free database[i].product for i >= newSize? Presumably you want to free all of them (but see realloc and code sample below).
Consider using
realloc()to resize an array. If it was successful your old will be retained.It's clumsy having client remember the previous database size, so consider creating a struct to old both the data and the size.
As you allocate fixed length string, it's easier to encode that in the type. Use constants (
PRODUCT_LEN) instead of hard-coding magic values.malloc()/realloc()of0bytes is implementation defined. My implementation returns a NULL so handling it as a special case.malloc()does not guarantee initialized memory so either initialize it after or use calloc.We don't cast void * (result from malloc) in C.
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define PRODUCT_LEN 100
typedef struct {
size_t size;
char (*products)[PRODUCT_LEN];
float *prices;
} database;
database *newDatabase() {
return calloc(1, sizeof(database));
}
database *resizeDatabase(database *db, size_t newSize) {
if(!db) return NULL;
database tmp;
tmp.products = realloc(db->products, newSize * sizeof *db->products);
if(newSize && !tmp.products) {
printf("realloc of products failed\n");
// handle error
return NULL;
}
db->products = tmp.products;
tmp.prices = realloc(db->prices, newSize * sizeof *db->prices);
if(newSize && !tmp.prices) {
printf("realloc of prices failed\n");
// handle error; we successfully resize products but failed
// to resize price. So we may need to shrink products but
// what if that now fails?
return NULL;
}
db->prices = tmp.prices;
for(size_t i = db->size; i < newSize; i ) {
db->products[i][0] = '\0';
db->prices[i] = -1;
}
db->size = newSize;
return db;
}
void freeDatabase(database *db) {
if(!db) return;
free(db->products);
free(db->prices);
free(db);
}
int main(void) {
database *db = newDatabase();
size_t tests[] = { 0, 1, 2, 1, 0 };
size_t tests_len = sizeof tests / sizeof *tests;
for(size_t i = 0; i < tests_len; i ) {
database *newDb = resizeDatabase(db, tests[i]);
if(!newDb) {
printf("resizefailed %zu\n", tests[i]);
return 1;
}
db = newDb;
}
freeDatabase(db);
}
and here is the valgrind run:
==533946== HEAP SUMMARY:
==533946== in use at exit: 0 bytes in 0 blocks
==533946== total heap usage: 9 allocs, 9 frees, 440 bytes allocated
==533946==
==533946== All heap blocks were freed -- no leaks are possible
CodePudding user response：
Newer version to compare to
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
typedef struct _product_t {
char *product;
float price;
} product_t;
product_t *newDatabase(product_t *database, int *dbSize, int newSize) {
for (int i = 0; i < *dbSize; i ) {
if (database[i].product != NULL) {
free(database[i].product);
}
}
// Free the previous memory allocated for the database
// Free the previous memory allocated for the database
free(database);
// Allocate new space for newSize products on the heap
database = (product_t*) malloc(newSize * sizeof(product_t));
// Initialize all products in the database with NULL and -1
for (int i = 0; i < newSize; i ) {
database[i].product = NULL;
database[i].price = -1;
}
// Update the database size
*dbSize = newSize;
// Return the pointer to the new database
return database;
}
int main(void) {
product_t *database = NULL;
int dbSize = 0;
char cmd;
do{
printf("Command?");
scanf(" %c", &cmd);
switch (cmd) {
case 'q':
printf("Bye!");
break;
case 'n':
printf("Size? ");
int newSize2 = 0;
scanf("%d", &newSize2);
if(newSize2 < 0) {
printf("Must be larger than 0");
break;
}
else{
database = newDatabase(database, &dbSize, newSize2);
break;
}
default:
printf("Unkown command '%c'\n",cmd);
}
}while(cmd != 'q');
return 0;
}