Insertion on a binary tree with strings in C

Question

I am having trouble with my insertion function into a binary tree of strings. I am doing this recursively. There are 2 compile errors that I cant seem to get rid of and it has to do with the names of the people. I will post my code and the errors that I have along with an example output and the file that I am supposed to read in. The display function was given to us by my professor to make it look a specific way. Thanks for the help in advance.

#include<stdio.h>
#include<stdlib.h>
#include<ctype.h>
#include<string.h>

#define MAX_NAME_LEN 25
#define START_DEPTH 0

typedef struct TreeNode_ {
    char name[MAX_NAME_LEN];
    struct TreeNode_ *left;
    struct TreeNode_ *right;
}TreeNode;

TreeNode* read_from_file(const char* file);
TreeNode* insert(TreeNode* node, const char *name);
TreeNode* create_node(const char *name);

int main (int argc, char *argv[]) {
    /*
     * Check command line parameters
     * */
    if (argc < 2) {
            printf("%s is missing parameters to run properly\n", argv[0]);
            return 1;
    }
    TreeNode* root = NULL;
    root = read_from_file(argv[1]);

    display_tree(root,START_DEPTH);
}

TreeNode* read_from_file(const char* file) {
    /* HINT
     *      TreeNode *t = NULL;
     *  t = insert(t,buffer);
     * loop check the return type on fscanf
     *  insert(t,buffer);
     */

    TreeNode *root = NULL;
    FILE *input = fopen(file, "r");
    char name[MAX_NAME_LEN];
    while(fscanf(input, "%s", &name) != EOF){
            //printf("\n%s", name);
            root = insert(root, name);
    }
    return root;
}

TreeNode* insert(TreeNode* node, const char *name) {

    if(node == NULL)
            node = create_node(name);

    else if( node != NULL){

            if(strcmp(name,node->name)<0)
                    node->left = insert(node->left, name);
            else if (strcmp(name,node->name) > 0)
                    node->right = insert(node->right,name);
    }
    return node;
}

TreeNode* create_node(const char *name) {

    TreeNode *node;
    node = (TreeNode*)malloc(sizeof(TreeNode));
    node->name = malloc(sizeof(TreeNode));;
    node->name = name;
    node->left = NULL;
    node->right = NULL;
    return node;
}


void padding (char ch, int n){

    int i;
    for (i = 0; i < n; i++)
            printf("%c%c%c%c", ch, ch ,ch, ch);

}

/*
 * A Beautiful way to display various sorts of trees, passing
 * this from one generation of students to the next.
 * */
void display_tree (TreeNode *root, int depth) {
    TreeNode *current = root;
    if (root == NULL) {
            padding (' ', depth);
            printf("-\n");
    }
else {
            display_tree(current->left, depth+1);
    padding(' ', depth);
    printf ( "%s\n", root->name);
    display_tree(current->right, depth+1);
}
}

The errors that I get are this, but keep in mind that I cut out a lot of code so these line numbers are wrong, the errors are in the create_node function when I assign the name to node->name.:

hw.c:167: error: incompatible types when assigning to type ‘char[25]’ from type ‘void *’
hw.c:168: error: incompatible types when assigning to type ‘char[25]’ from type ‘const char *’

The names that I read in (names.txt)

matt
sue
erik
nick
james
sachin
bob

And the desired output is as follows:

  -
        bob
            -
    erik
            -
        james
            -
matt
            -
        nick
                -
            sachin
            -
    sue
        -

Answer 1

In the line

while(fscanf(input, "%s", &name)

it should be name rather than &name. (But on modern systems your version is likely to appear to work). But you should specify a maximum length for fscanf - otherwise this line is as bad as gets.

In this code there's a problem:

node->name = malloc(sizeof(TreeNode));;
node->name = name;

What do you think = is doing? It should be clear from comparing these two lines that they can't both be correct.

The actual effect of = is that the value of the object denoted by the left-hand-side is replaced with the value on the right-hand-side.

On the first line you set node->name to hold the address of some freshly allocated memory. But on the second line you set it to hold the same address which name currently holds. So it no longer holds the address of the freshly allocated memory.

Instead, what you want to do is to copy the contents of the string pointed to by name into the freshly-allocated memory. There is a function for copying strings:

strcpy(node->name, name);

Answer 2

You cannot assign strings using:

node->name = name;

you need to use functions such as strcpy, strncpy, strcat, strncat for that:

strcpy(node->name, name); // Unsafe. name could be too long.

strncpy(node->name, name, MAX_NAME_LEN);
node->name[MAX_NAME_LEN] = '\0';

node->name[0] = '\0`;
strcat(node->name, name);  // Unsafe. name could be too long.

node->name[0] = '\0`;
strncat(node->name, name, MAX_NAME_LEN);
node->name[MAX_NAME_LEN] = '\0';