Structuring leafs in a hierarchical tree

Question

This code fills a tree with values based on their depths. But when traversing the tree, I cannot manage to determine the actual number of children without iterating over the parent node. This is necessary because the subleafs are stored in in the node underneath the current one. Which conceptual changes are necessary to store the leafs directly within the current node?

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

#ifndef NULL
#define NULL ((void *) 0)
#endif

// ----

typedef struct _Tree_Node {
    // data ptr
    void *p;

    // number of nodes
    int cnt;
    struct _Tree_Node **nodes;

    // parent nodes
    struct _Tree_Node *parent;
} Tree_Node;

typedef struct {
    Tree_Node root;
} Tree;

void Tree_Init(Tree *this) {
    this->root.p = NULL;
    this->root.cnt = 0;
    this->root.nodes = NULL;
    this->root.parent = NULL;
}

Tree_Node* Tree_AddNode(Tree_Node *node) {
    if (node->cnt == 0) {
        node->nodes = malloc(sizeof(Tree_Node *));
    } else {
        node->nodes = realloc(
            node->nodes,
            (node->cnt + 1) * sizeof(Tree_Node *)
        );
    }

    Tree_Node *res
        = node->nodes[node->cnt]
        = malloc(sizeof(Tree_Node));

    res->p = NULL;
    res->cnt = 0;
    res->nodes = NULL;
    res->parent = node;

    node->cnt++;

    return res;
}

// ----

void handleNode(Tree_Node *node, int depth) {
    int j = depth;

    printf("\n");

    while (j--) {
        printf("    ");
    }

    printf("depth=%d ", depth);

    if (node->p == NULL) {
        goto out;
    }

    int cnt = 0;
    for (int i = 0; i < node->parent->cnt - 1; i++) {
        if (node->parent->nodes[i] == node) {
            cnt = node->parent->nodes[i + 1]->cnt;
        }
    }

    printf("value=%s cnt=%i", node->p, cnt);

out:
    for (int i = 0; i < node->cnt; i++) {
        handleNode(node->nodes[i], depth + 1);
    }
}

Tree tree;

int curdepth;
Tree_Node *curnode;

void add(int depth, char *s) {
    printf("%s: depth (%d) > curdepth (%d): %d\n", s, depth, curdepth, depth > curdepth);

    if (depth > curdepth) {
        curnode = Tree_AddNode(curnode);

        Tree_Node *node = Tree_AddNode(curnode);

        node->p = malloc(strlen(s) + 1);
        memcpy(node->p, s, strlen(s) + 1);

        curdepth++;
    } else {
        while (curdepth - depth > 0) {
            if (curnode->parent == NULL) {
                printf("Illegal nesting\n");
                return;
            }

            curnode = curnode->parent;
            curdepth--;
        }

        Tree_Node *node = Tree_AddNode(curnode);

        node->p = malloc(strlen(s) + 1);
        memcpy(node->p, s, strlen(s) + 1);
    }
}

void main(void) {
    Tree_Init(&tree);

    curnode = &tree.root;
    curdepth = 0;

    add(0, "1");
    add(1, "1.1");
    add(2, "1.1.1");
    add(3, "1.1.1.1");
    add(4, "1.1.1.1.1");
    add(4, "1.1.1.1.2");
    add(4, "1.1.1.1.3");
    add(4, "1.1.1.1.4");
    add(2, "1.1.2");
    add(0, "2");

    handleNode(&tree.root, 0);
}

It isn't clear what you're trying to achieve. Please elaborate and provide an example — Eli Bendersky, Mar 23 '10 at 19:39
Sorry, I thought it was quite clear what I meant. Well, the problem was that in handleNode(), I was unable to figure out the number of (parent) nodes the current leaf has. node->parent->cnt seemed obvious to me but even after adding a if (node->parent != NULL), Valgrind gave me lots of warnings and the printf() also printed the wrong numbers. That's probably because after the realloc(), the pointers aren't showing to the same node anymore, as Giuseppe pointed out. — user206268, Mar 23 '10 at 22:56
You should not use "this" or redefine "NULL" in that manner... it could break things if you decide to use it with a C++ compiler. — Michael Aaron Safyan, Mar 24 '10 at 09:17
The project is bound to C. So it shouldn't be a problem. NULL is not defined by default. Even some glibc classes do (re-)define NULL. — user206268, Mar 24 '10 at 09:54
@timn, if you redefine NULL, you should define it as 0 or 0L, and not as ((void*) 0), as doing so is an incompatible definition. — Michael Aaron Safyan, Mar 24 '10 at 13:55
No, it's allowed in ANSI C (see also http://c-faq.com/null/safermacs.html). — user206268, Mar 24 '10 at 17:35

score 1 · Answer 1 · answered Mar 23 '10 at 21:09

I see two problems in you program

1) When you "realloc" the node list, you actually move in memory the node objects, so the parent pointer in their children must me updated as well. I suggest you to transform the array of nodes into an array of pointers to nodes, so you can realloc it without correcting pointers.

2) You forgot to terminate strings:

  node->p = malloc(strlen(s));
  memcpy(node->p, s, strlen(s));

should be:

  node->p = malloc(strlen(s)+1);
  memcpy(node->p, s, strlen(s)+1);

or also simply

  node->p = strdup(s);

Maybe there are other issues, but I strongly suggest to correct these ones first. I hope it may help you Regards

1) Oh, alright. I didn't think of that. Now I understand why Valgrind pointed realloc(). Unbelievable I spent so many hours looking for that bug while looking in the wrong place. 2) Sorry, in the real code I wasn't using strings but structs so this wasn't really the culprit. Anyway, the reason I did not go with the array of pointers in the first place was that I feared that they were more inefficient than a large chunk being resized all the time. — user206268, Mar 23 '10 at 22:51

Michael Aaron Safyan · Answer 2 · 2010-03-24T09:32:56.820

1

If your structure is truly a tree, then the following pseudo code for recursively counting nodes may help:

def total_number_of_leaf_nodes(node):
    if node does not have children:
        return 1
    else:
        sum = 0
        for each child of node:
            sum += total_number_of_leaf_nodes(child)
        return sum

If it is at all possible for you to use C++, then I would strongly advise it. Being able to use an std::vector or std::list to store your child nodes and being able to make the data element have a template type would greatly simplify the complexity of your code.

edited Mar 24 '10 at 09:32

answered Mar 24 '10 at 09:27

Michael Aaron Safyan

93,612
16
138
200

Exactly. That's what my remaining problem is. It is down to the part where depth > curdepth is true because it adds a node on the current level and only then goes a level deeper. And that's also the reason why the number of leafs is stored in currentNode->parent->nodes[indexOfCurrentNode + 1]->cnt. How can my current approach be improved to match your 'structure'? – user206268 Mar 24 '10 at 10:04

Structuring leafs in a hierarchical tree

2 Answers2