There are two problems with your insert
function.
- It should be passed a double pointer to a
struct node
if you plan to mutate the pointer, otherwise it should bereturn
ing on each recursive call if you plan to use a single pointer to astruct node
. - You are not
malloc
ing memory for the word when creating a new node.
To find problems in other parts of your code, use valgrind
(here). It's a fantastic tool for debugging memory leaks or segmentation fault errors.
To solve problem 1, I will show the example of passing a single pointer and return
ing (still mutating). Your insert function (with problem 2 solved) should look as follows:
node *insert( node *item, char *word ) {
if ( item == NULL ) {
node *new_item = malloc( sizeof( struct node ) );
new_item->word = malloc( sizeof( char ) * ( strlen( word ) + 1 ) ); // Note, this line (p2).
strcpy( new_item->word, word );
new_item->count = 1; // << Note change here.
new_item->left = NULL;
new_item->right = NULL;
return new_item;
} else {
int cmp_result = strcmp( word, item->word );
if ( cmp_result < 0 ) {
item->left = insert( item->left, word );
item->count++;
} else if ( cmp_result > 0 ) {
item->right = insert( item->right, word );
item->count++;
} else {
// Node already exists, do what you see fit here.
}
}
return item;
}
To solve problem 2, the error lies within the block of code that creates a new node. Looking here:
item = ( node* )malloc( sizeof( node ) );
strcpy( item->word, word ); // << Here, invalid (error).
... you are not malloc
ing a block of memory for the word. What you are doing is overwriting memory inside your structure and possibly to other memory addresses you have not allocated (depending on when the garbage value will be 0 to simulate a NULL
terminator). This is undefined behaviour.
The solution is to do the following:
item = ( node* ) malloc( sizeof( node ) );
item->word = malloc( sizeof( char ) * ( strlen( word ) + 1 ) ); // << Fix.
strcpy( item->word, word ); // << Now, valid.
... note the + 1
to ensure that there is room for the NULL
terminator since strlen
returns the string length of the array of characters passed to it.
Remark:
- It's also not a good idea to cast the result of
malloc
, but it's entirely up to you as it is not causing an error (but could make an error message not appear when it should). - It's also important that your main function has a void parameter type,
main( void )
notmain( )
unless you intend to use the feature.