2

I'm writing a program that given a root of a huffman tree, runs through the tree and gives me back an array of Symbols which represents the frequency of each letter in the tree.
Symbol is defined like this:

typedef struct {
    char chr;
    int counter;
} Symbol;

And each huffman node is defined like this:

struct HNode;
typedef struct HNode {
    char chr;
    struct HNode *left, *right;
} HNode;

I don't have an add() function for a huffman tree, so I made it manually like this:

int main()
{
    Symbol * result;
    HNode root, left, right, leftleft, leftright, rightleft, rightright, leftleftleft, leftleftright;
    root.chr = '\0';
    left.chr = '\0';
    right.chr = '\0';
    leftleft.chr = '\0';
    leftleftleft.chr = 'c';
    leftleftright.chr = 'd';
    leftright.chr = 'e';
    rightleft.chr = 'b';
    rightright.chr = 'a';
    root.left = &left;
    root.right = &right;
    left.left = &leftleft;
    left.right = &leftright;
    right.left = &rightleft;
    right.right = &rightright;
    leftleft.left = &leftleftleft;
    leftleft.right = &leftleftright;
    leftleftleft.left = NULL;
    leftleftleft.right = NULL;
    leftleftright.left = NULL;
    leftleftright.right = NULL;
    leftright.left = NULL;
    leftright.right = NULL;
    rightleft.left = NULL;
    rightleft.right = NULL;
    rightright.left = NULL;
    rightright.right = NULL;    
    result = getSL(&root);
    while (result->chr != '\0')
    {
        printf("%c : %d\n", result->chr, result->counter);
        result++;
    }
    getchar();
    return 0;
}

The tree looks like this:My huffman tree

The function itself runs recursively through the tree and adds each element to a dynamically allocated array of Symbols:

Symbol * getSL(HNode * root)
{
    int length;
    Symbol * s, *scopy;
    length = 0;
    s = (Symbol *)calloc((2 + length) * sizeof(Symbol *), 1);
    if (!root) return NULL;
    if (root->left && root->right)
    {
        Symbol *s0, *s1;
        int s0Length, s1Length;
        s = (Symbol *) realloc(s, (2 + length) * sizeof(Symbol *));
        s0 = getSL(root->left);
        s1 = getSL(root->right);
        s0Length = 0;
        while ((s0 + s0Length)->chr != '\0')
        {
            s = (Symbol *)realloc(s, (2 + length) * sizeof(Symbol *));
            (s + length)->counter = (s0 + s0Length)->counter + 1;
            (s + length)->chr = (s0 + s0Length)->chr;
            length++;
            s0Length++;
        }
        s1Length = 0;
        while ((s1 + s1Length)->chr != '\0')
        {
            s = (Symbol *)realloc(s, (2 + length) * sizeof(Symbol *));
            (s + length)->counter = (s1 + s1Length)->counter + 1;
            (s + length)->chr = (s1 + s1Length)->chr;
            length++;
            s1Length++;
        }
        (s + length)->chr = '\0';
    }
    else
    {
        s->chr = root->chr;
        s->counter = 0;
        length++;
        (s + length)->chr = '\0';
    }

    scopy = s;
    while (scopy->chr != '\0')
    {
        printf("%c : %d\n", scopy->chr, scopy->counter);
        scopy++;
    }
    printf("----------------------------------------------\n");
    return s;
}

Note: It will be much easier to analyze if you run the program in debug mode as I have added a loop that runs through the array after each phase in the recursion.

The problem comes in this realloc:

s1Length = 0;
        while ((s1 + s1Length)->chr != '\0')
        {
            s = (Symbol *)realloc(s, (2 + length) * sizeof(Symbol *));
            (s + length)->counter = (s1 + s1Length)->counter + 1;
            (s + length)->chr = (s1 + s1Length)->chr;
            length++;
            s1Length++;
        }

It doesn't happen in one phase, but in the final phase, it does. It says it triggered a breakpoint and if I try to continue, it crashes.

I have absolutely no idea what's wrong and I'll really appreciate your help.

Thank you very much in advance.

shoham
  • 792
  • 2
  • 12
  • 30
  • I copied the same code and ran it. Didnt get any issue. Can you explain or paste what exactly is the error that you are getting. – pankaj Jul 06 '14 at 14:50
  • @user3121023 Thank you very very very very very very very much. I'm working on this for 5 hours and I didn't notice that. – shoham Jul 06 '14 at 15:00

1 Answers1

2

You don't allocate enough memory for s. You seem to want s to point to an array of Symbols, but you only allocate space for an array of Symbol*:

s = (Symbol *)realloc(s, (2 + length) * sizeof(Symbol *));

This allocates enough space for several pointers, but the result of the realloc is used as if it would point to space for several Symbol structs:

(s + length)->counter = (s1 + s1Length)->counter + 1;

If sizeof(Symbol) > sizeof(Symbol*) this means you'll likely write past the allocated space, corrupting memory.

Probably you wanted to use sizeof(Symbol) in your allocations instead:

s = (Symbol *)realloc(s, (2 + length) * sizeof(Symbol));

(Additionally, casting the return value of calloc/realloc is unnecessary and I'm not sure why you allocate 2+length elements - 1+length seem to be enough.)

sth
  • 222,467
  • 53
  • 283
  • 367