You may be making things much harder on yourself than they need to be. For starters, you can always reduce the effort required to keep track of your currently allocated number of table
structs, by adding a simply wrapper around your data that includes a counter. You can also use typedef
here to your advantage.
For example, instead of handling table
directly, why not move the elements of table
to a simple struct of its own, say tbl_data
and simply include a pointer in table
to the data and a counter for the number of tbl_data
elements allocated? It could work something like this:
typedef struct {
char key[MAX_KEY_LENGTH];
int val;
} tbl_data;
typedef struct { /* simple wrapper preserving allocation size in ntbl */
tbl_data *tbl;
size_t ntbl;
} table;
Next, avoid using global variables. You are already passing **tab
as a parameter, there is no need to declare insTab
as a global. Declare it in main()
as pass it as a parameter as it appears you intended to do. (note: using the wrapper, you can use a simple static instance of table
and pass it as *tab
)
Think about the structure scheme above. You are only allocating for each tbl_data
added, and after allocating and validating the new memory, you simply assign your temp
to tab->tbl[tab->ntbl]
and increment the number of tbl_data
(tab->ntbl
) by 1. Much simpler. And since you are increasing the allocation by 1
with each addFieldToTable
, you can just use realloc
for the first and all subsequent allocations simplifying addFieldToTable
.
(note: repeatedly calling malloc
(or realloc
or calloc
) for every element added is not a very efficient allocation scheme, but it is fine for now. You can allocate blocks of 32
, 512
and realloc
when tbl->ntbl
reaches the limit, or just double tbl->ntbl
each time and realloc
when your reach the new limit to reduce the realloc
calls)
If you declare table insTab = { NULL, .ntbl = 0 };
in main()
, your addFieldToTable
with improved validation can be reduced to:
void addFieldToTable (table *tab, char *key, int val)
{
if (!key) { /* validate key is not NULL */
fprintf (stderr, "error: key parameter is NULL\n");
return;
}
tbl_data temp;
size_t keylen = strlen (key); /* get the length of key */
if (keylen > MAX_KEY_LENGTH) { /* truncate if > MAX_KEY_LENGTH */
key[keylen - 1] = 0;
fprintf (stderr, "warning: key exceeds %d, truncated.\n",
MAX_KEY_LENGTH);
}
strcpy (temp.key, key);
temp.val = val;
/* just realloc tbl_data, since you are increasing by 1 each time */
void *tmptbl = realloc (tab->tbl, (tab->ntbl + 1) * sizeof *(tab->tbl));
if (!tmptbl) { /* validate realloc succeeded or handle error */
fprintf (stderr, "error: memory exhausted at '%zu' elements.\n",
tab->ntbl);
return;
}
tab->tbl = tmptbl; /* assign new block to *tab */
tab->tbl[tab->ntbl] = temp; /* assign temp */
tab->ntbl++; /* increment ntbl */
}
(note: you should really change your function type to tbl_data*
and return a pointer to tab->tbl
so you can (1) return an indication of success/failure, e.g. the pointer or NULL
; and (2) make a pointer to the new tbl_data
immediately available. You could also just declare it as type int
and return 1
for success and 0
on failure and keep the benefits of (1) above)
Putting it altogether, you could do something similar to the following:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define MAX_KEY_LENGTH 20
typedef struct {
char key[MAX_KEY_LENGTH];
int val;
} tbl_data;
typedef struct { /* simple wrapper preserving allocation size in ntbl */
tbl_data *tbl;
size_t ntbl;
} table;
void addFieldToTable (table *tab, char *key, int val)
{
if (!key) { /* validate key is not NULL */
fprintf (stderr, "error: key parameter is NULL\n");
return;
}
tbl_data temp;
size_t keylen = strlen (key); /* get the length of key */
if (keylen > MAX_KEY_LENGTH) { /* truncate if > MAX_KEY_LENGTH */
key[keylen - 1] = 0;
fprintf (stderr, "warning: key exceeds %d, truncated.\n",
MAX_KEY_LENGTH);
}
strcpy (temp.key, key);
temp.val = val;
/* just realloc tbl_data, since you are increasing by 1 each time */
void *tmptbl = realloc (tab->tbl, (tab->ntbl + 1) * sizeof *(tab->tbl));
if (!tmptbl) { /* validate realloc succeeded or handle error */
fprintf (stderr, "error: memory exhausted at '%zu' elements.\n",
tab->ntbl);
return;
}
tab->tbl = tmptbl; /* assign new block to *tab */
tab->tbl[tab->ntbl] = temp; /* assign temp */
tab->ntbl++; /* increment ntbl */
}
int main (void)
{
table insTab = { NULL, .ntbl = 0 };
addFieldToTable (&insTab, "mov", 0);
addFieldToTable (&insTab, "cmp", 1);
addFieldToTable (&insTab, "next command", 1234);
if (insTab.ntbl == 0) {
fprintf (stderr, "error: insTab is empty.\n");
exit (EXIT_FAILURE);
}
for (size_t i = 0; i < insTab.ntbl; i++)
printf ("%-*s : %d\n", MAX_KEY_LENGTH,
insTab.tbl[i].key, insTab.tbl[i].val);
free (insTab.tbl); /* don't forget to free allocated memory */
return 0;
}
Example Use/Output
$ ./bin/struct_table
mov : 0
cmp : 1
next command : 1234
Memory Use/Error Check
In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.
It is imperative that you use a memory error checking program to insure you do not attempt to write beyond/outside the bounds of your allocated block of memory, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.
For Linux valgrind
is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.
$ valgrind ./bin/struct_table
==4003== Memcheck, a memory error detector
==4003== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==4003== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==4003== Command: ./bin/struct_table
==4003==
mov : 0
cmp : 1
next command : 1234
==4003==
==4003== HEAP SUMMARY:
==4003== in use at exit: 0 bytes in 0 blocks
==4003== total heap usage: 3 allocs, 3 frees, 144 bytes allocated
==4003==
==4003== All heap blocks were freed -- no leaks are possible
==4003==
==4003== For counts of detected and suppressed errors, rerun with: -v
==4003== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Always confirm that you have freed all memory you have allocated and that there are no memory errors.
Look things over and let me know if you have any questions.