I can't seem to insert a node in a tree. Can someone take a look at my code and tell me where did I do wrong?

Question

I've been trying to insert a node to my tree through file processing but it only always inserts one node then the program will force close itself. I have tried to put the data on a linkedlist and it worked. So probably the problem here is not the file.txt itself but my tree. Can someone tell me where did i do wrong?

Here is my tree algorithm

struct user {
    char username[50];
    int money;
    char favorite[50];
    user *left;
    user *right;
};

user *root = NULL;

user *userTree(char username[], int money, char favorite[]) {
    user *newBranch = (user *) malloc(sizeof(user));
    strcpy(newBranch->favorite, favorite);
    strcpy(newBranch->username, username);
    newBranch->money = money;
    return newBranch;
}

user *insertUser(user *root,char username[], int money, char favorite[]) {
    if (root == NULL) {
        return userTree(username ,money, favorite);
    }
    else if (strcmp(username, root->username) < 0) {
        root->left = insertUser(root->left, username, money, favorite);
    }
    else {
        root->right = insertUser(root->right, username, money, favorite);
    }
    return root;
}

Here is my file processing algorithm

void insertCredential() {
    FILE* fr = fopen("users/users.txt", "r");
    while (!feof(fr)) {
        char username[50], garbage[50], favorite[50];
        int money;
        fscanf(fr, "%[^#]#%[^#]#%d#%[^\n]\n", username, garbage, &money, favorite);
        root = insertUser(root, username, money, favorite);
        puts("test insert");
    }
    fclose(fr);
}

I have tried to put the data in a linked list and it worked but I want to know exactly why can't I insert it on a tree

There are some indicators that you're not programming C, but rather **C++**. Like being able to use the structure name `user` without the `struct` keyword. Or the need to cast the result of `malloc` (which you [shouldn't do in C](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858)). — Some programmer dude, Nov 13 '22 at 11:59
Your code doesn't initialise `left` and `right` members after `malloc()`, leading to undefined behavior. — trincot, Nov 13 '22 at 12:04
[Why is “while( !feof(file) )” always wrong?](https://stackoverflow.com/q/5431941/12149471) — Andreas Wenzel, Nov 13 '22 at 12:09
@trincot yeah it works after I initialize the left and right to be NULL, thank you so much! — Arigatod, Nov 13 '22 at 12:11
That code, as shown, should not build as plain C. That means you're either build it as C++ (is the source files named something ending in `.cpp` or `.cc` or similar?) or it's not a proper [mre]. — Some programmer dude, Nov 13 '22 at 12:16
Then it's built as a C++ source file. If you are studying C then you're not learning it correctly. A plain C source file should have the ending `.c`. — Some programmer dude, Nov 13 '22 at 12:24
Arigatod, Tip: useful to also post sample data used for input as well as output seen and output expected. — chux - Reinstate Monica, Nov 13 '22 at 12:40

chux - Reinstate Monica · Accepted Answer · 2022-11-13T12:43:58.057

At least these problems:

code doesn't initialise left and right members after malloc(), leading to undefined behavior.. Consider calloc(). @trincot
while (!feof(fr)) is wrong. @Andreas Wenzel. Check return value of fscanf().
"%[^#]#%[^#]#%d#%[^\n]\n" lacks widths allowing input overruns. Add widths. "%49[^#]#%49[^#]#%d#%49[^\n]\n"
Result of fopen() not checked.
Allocation retuned pointer not checked for success.
Code is not valid C. Looks like a C++ compiler used: user *root = NULL; --> struct user *root = NULL;. @Some programmer dude

Weaknesses:

money as an int may be too narrow. Consider long and a width: "%d" --> "%9ld".
Cast not needed in newBranch = (user *) malloc(sizeof(user)); in C. Easier to maintain when the size referenced the object and not the type: newBranch = malloc(sizeof newBranch[0]);
Greater applicability to use const for unchanged reference data: userTree(char username[], int money, char favorite[]) --> userTree(const char username[], int money, const char favorite[]).
Trailing "\n" can read multiples white-spaces. Better error checking to use fgets() to read 1 line of line oriented data and then parse.

Note that even with `"%49[^#]#%49[^#]#%d#%49[^\n]\n"`, you can still trigger undefined behavior with the `%d`. The only reliable format string must also limit `%d`, but the best way to use `scanf` is to toss it in the bin. — William Pursell, Nov 13 '22 at 12:51
@WilliamPursell Agree about `"%d"` and `scanf()`. See updated #10. — chux - Reinstate Monica, Nov 13 '22 at 12:52

I can't seem to insert a node in a tree. Can someone take a look at my code and tell me where did I do wrong?

1 Answers1