binary search tree problem

[sugaray]

the binary search tree node here contains another structure as it's
data field,
programs did successfully work when data field is int, char, this time
i got stucked, don't know why ? if there's something to do with
dynamic data object ?
thanx for your help.

================== begin of code ==============================
#include <stdio.h>
#include <stdlib.h>

typedef struct tagStock
{
char name[64];
int tag;
} Stock;

typedef struct tagBinarySearchTreeNode BSTNode;
struct tagBinarySearchTreeNode
{
Stock *stock;
BSTNode *left;
BSTNode *right;
};

typedef struct tagBinarySearchTree
{
size_t size;
BSTNode *root;
} BSTree;

Stock *
CreateStock (char *name, int tag)
{
Stock *stock;

stock = (Stock *) malloc (sizeof (Stock));
if (stock == NULL)
return NULL;

strcpy (stock->name, name);
stock->tag = tag;

return stock;
}

BSTNode *
InsertNode (BSTree * tree, Stock * stock)
{
BSTNode *tmp;
BSTNode *root = tree->root;
if (root == NULL)
{
root->stock = stock;
root->left = NULL;
root->right = NULL;
tree->size = 1;
}
else
{
while (root != NULL)
{
tmp = root;
if ((stock->tag) < (root->stock->tag))
root = root->left;
else
root = root->right;
}

if ((stock->tag) < (tmp->stock->tag))
{
root->stock = stock;
root->left = NULL;
root->right = NULL;
tmp->left = root;
tree->size++;
}
else if ((stock->tag) > (tmp->stock->tag))
{
root->stock = stock;
root->left = NULL;
root->right = NULL;
tmp->right = root;
tree->size++;
}
else return NULL;
}
return root;
}

void
Traversal (BSTNode * root)
{
if (root == NULL)
return;
Traversal (root->left);
printf ("%d\n", root->stock->tag);
Traversal (root->right);
}

int
main (int argc, char **argv)
{
char name[64];
int tag = 0, i;
Stock *stock;
BSTree *tree = NULL;

for (i = 0; i < 10; ++i)
{
scanf ("%s", name);
scanf ("%d", &tag);
stock = CreateStock (name, tag);
InsertNode (tree, stock);
}

Traversal (tree->root);

return 0;
}
======================== end of code
=========================================

 

[Christian Bau]

<snipped>

In function InsertNode, you have a loop

while (root != NULL) { ... }

Since there is no break statement in the loop, you will have root ==
NULL after executing this loop. However, in the following code you have
assignments to root->xxx - that's a bad idea.

 

[Richard Bos]

the binary search tree node here contains another structure as it's
data field,
programs did successfully work when data field is int, char,

By accident.

this time i got stucked, don't know why ? if there's something to do with
dynamic data object ?

It's nothing to do with your struct member and everything with your
tree.

InsertNode (BSTree * tree, Stock * stock)
{
BSTNode *tmp;
BSTNode *root = tree->root;

This function asks for tree->root without checking whether tree points
at a valid tree object...

BSTree *tree = NULL;

InsertNode (tree, stock);

....yet in your main function, you pass it a null pointer. That this
worked before is mere accident. Correct InsertNode() so that it checks
for null pointers, and does something about them.

if (root == NULL)
{
root->stock = stock;
root->left = NULL;
root->right = NULL;
tree->size = 1;
}

And while you're at it, it would be a very good idea not to
_intentionally_ write through a null pointer. You do this throughout
your insertion function. Where did you think all those nodes would end
up? They aren't assigned some memory by magic, you know.

Richard

 

[CBFalconer]

the binary search tree node here contains another structure as
it's data field,
programs did successfully work when data field is int, char, this
time i got stucked, don't know why ? if there's something to do
with dynamic data object ?
thanx for your help.

================== begin of code ==============================
#include <stdio.h>
#include <stdlib.h>

typedef struct tagStock
{
char name[64];
int tag;
} Stock;

typedef struct tagBinarySearchTreeNode BSTNode;
struct tagBinarySearchTreeNode
{
Stock *stock;
BSTNode *left;
BSTNode *right;
};

typedef struct tagBinarySearchTree
{
size_t size;
BSTNode *root;
} BSTree;

Stock *
CreateStock (char *name, int tag)
{
Stock *stock;

stock = (Stock *) malloc (sizeof (Stock));
if (stock == NULL)
return NULL;

strcpy (stock->name, name);
stock->tag = tag;

return stock;
}

BSTNode *
InsertNode (BSTree * tree, Stock * stock)
{
BSTNode *tmp;
BSTNode *root = tree->root;
if (root == NULL)
{
root->stock = stock;
root->left = NULL;
root->right = NULL;
tree->size = 1;
}

I stopped right here. If root is NULL, how can root possibly
point to any fields whatsoever?

 

[sugaray]

after a bit of modification of the faults everybody pointed out,
it still can't work correctly. i'm kinda in the middle of nowhere right
now.

BSTNode *InsertNode (BSTree * tree, Stock * stock)
{
BSTNode **tmp;
BSTNode *root;

if(tree!=NULL)
root=tree->root;

if (root == NULL)
{
root->stock = stock;
root->left = NULL;
root->right = NULL;
tree->size = 1;
}
else
{
while (root != NULL)
{
tmp = &root;
if ((stock->tag) < (root->stock->tag))
root = root->left;
else
root = root->right;
}

if ((stock->tag) < ((*tmp)->stock->tag))
{
root->stock = stock;
root->left = NULL;
root->right = NULL;
(*tmp)->left = root;
tree->size++;
}
else if ((stock->tag) > ((*tmp)->stock->tag))
{
root->stock = stock;
root->left = NULL;
root->right = NULL;
(*tmp)->right = root;
tree->size++;
}
else return NULL;
}
return root;
}

int main (int argc, char **argv)
{
char name[64];
int tag = 0, i;
Stock *stock;
BSTree *tree;
tree=(BSTree *)malloc(sizeof(BSTree));
assert(tree!=NULL);
tree->root=NULL;
tree->size=0;

for (i = 0; i < 10; ++i)
{
scanf ("%s", name);
scanf ("%d", &tag);
stock = CreateStock (name, tag);
InsertNode (tree, stock);
}

Traversal (tree->root);

free(tree);

return 0;
}

 

[Barry Schwarz]

after a bit of modification of the faults everybody pointed out,
it still can't work correctly. i'm kinda in the middle of nowhere right
now.

BSTNode *InsertNode (BSTree * tree, Stock * stock)
{
BSTNode **tmp;
BSTNode *root;

if(tree!=NULL)
root=tree->root;

if (root == NULL)

If tree is NULL, root is uninitialized and this invokes undefined
behavior.

{
root->stock = stock;

Since root is NULL, you cannot dereference as you try to do here.
This invokes undefined behavior.

root->left = NULL;
root->right = NULL;
tree->size = 1;

Since you check for tree being NULL earlier, it is possible that it is
NULL here also.

}
else
{
while (root != NULL)
{
tmp = &root;

&root is a constant. Why is this assignment inside the loop?

if ((stock->tag) < (root->stock->tag))
root = root->left;
else
root = root->right;
}

if ((stock->tag) < ((*tmp)->stock->tag))

(*tmp) will always evaluate to root. What is accomplished by this
additional level of indirection?

{
root->stock = stock;
root->left = NULL;
root->right = NULL;
(*tmp)->left = root;
tree->size++;
}
else if ((stock->tag) > ((*tmp)->stock->tag))
{
root->stock = stock;
root->left = NULL;
root->right = NULL;
(*tmp)->right = root;

These two assign values to the same variable.

Do you really want root->right pointing to the same struct that root
points to? The list will become circular.

tree->size++;
}
else return NULL;
}
return root;
}

int main (int argc, char **argv)
{
char name[64];
int tag = 0, i;
Stock *stock;
BSTree *tree;
tree=(BSTree *)malloc(sizeof(BSTree));

Don't cast the return from malloc. It cannot help in this case but
will suppress a useful diagnostic if you forget to include stdlib.h.

assert(tree!=NULL);
tree->root=NULL;
tree->size=0;

for (i = 0; i < 10; ++i)
{
scanf ("%s", name);
scanf ("%d", &tag);
stock = CreateStock (name, tag);
InsertNode (tree, stock);
}

Traversal (tree->root);

free(tree);

return 0;
}

<<Remove the del for email>>

 

[CBFalconer]

after a bit of modification of the faults everybody pointed out,
it still can't work correctly. i'm kinda in the middle of
nowhere right now.

You top-posted this as a reply to a message of mine, and totally
ignored the fundamental error I pointed out. Search google for
Richard Heathfields course on reading.

 

[Richard Bos]

after a bit of modification of the faults everybody pointed out,
it still can't work correctly.

You did not correct _any_ of the errors I pointed out to you. You did,
indeed, only "modify the faults"; the faults are still there, but in
some cases you've muddled the code by introducing an extra pointer and
leaving the original error intact. You still do not allocate any memory
for your nodes.
Before you continue trying to create dynamic data types, go back to the
chapter which explains allocated memory (i.e., malloc(), realloc(),
calloc() and free()). Do not go back to your tree until you _fully_
understand memory allocation. Reading it through once is not enough.
Until you understand completely what malloc() is for and how pointers
work (and more importantly, how they do not work), you will never write
a working binary tree implementation.

Richard