I've decided to go through all of D&R's exercises and in doing so I have encountered a peculiar event. Based on the book's own addtree function I modified it for my own structure:

struct gnode {
    char **words;
    int count;
    struct gnode *left;
    struct gnode *right;
};

And it now is:

struct gnode *addtree(struct gnode *p, char *w) {
    int cond;

    if (p == NULL) {
        printf("init null node\n");
        p = (struct gnode *)malloc(sizeof(struct gnode));  
        //MISTAKE I WAS MAKING:
        // struct gnode *p =(struct gnode *)malloc(sizeof(struct gnode)); 
        //p would be NULL every time.
        p->count = 1;
        p->words = malloc(8); 
        p->words[0] = strdup2(w);
        p->left = p->right = NULL;
    } else
    if ((cond = compare(w, p->words[0])) == 0) {
        printf("comp hit\n");
        p->count  ;
        p->words = realloc(p->words, p->count * 8);
        p->words[p->count] = strdup2(w);
    } else
    if (cond < 0)
        p->left = addtree(p->left, w);
    else
        p->right = addtree(p->right, w);

    return p;
}

I would like to know why if a local pointer with the same name as the argument is returned, it is NULL every time.

CodePudding user response：

There are multiple issues in your code:

• when the tree is empty, the line in the original code struct gnode *p = (struct gnode *)malloc(sizeof(struct gnode)); allocates a new gnode object, but also defines a new identifier p with a local scope, effectively shadowing the argument name. Hence the argument variable p is not updated and ultimately the original value (a null pointer) is returned by return p; at the end of the function.

  You can prevent this type of silly mistake by increasing the compiler warning level: gcc -Wall -Wextra or clang -Weverything

• the allocation p->words = malloc(8); is not portable. You should use:

  p->words = malloc(sizeof(*p->words));
  

• same for p->words = realloc(p->words, p->count * 8), it should be

  p->words = realloc(p->words, p->count * sizeof(*p->words));
  

• furthermore, since p->count was already incremented, setting the duplicate word should use:

  p->words[p->count - 1] = strdup2(w);
  

• why use strdup2(w) instead of strdup(w)?

• w should probably be defined as const char *w.

Here is a modified version:

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

struct gnode {
    char **words;
    int count;
    struct gnode *left;
    struct gnode *right;
};

struct gnode *addtree(struct gnode *p, const char *w) {
    int cond;

    if (p == NULL) {
        printf("init null node\n");
        p = malloc(sizeof(*p));  
        p->count = 1;
        p->words = malloc(sizeof(*p->words)); 
        p->words[0] = strdup2(w);
        p->left = p->right = NULL;
    } else
    if ((cond = compare(w, p->words[0])) == 0) {
        printf("comp hit\n");
        p->count  ;
        p->words = realloc(p->words, p->count * sizeof(*p->words));
        p->words[p->count - 1] = strdup2(w);
    } else
    if (cond < 0) {
        p->left = addtree(p->left, w);
    } else {
        p->right = addtree(p->right, w);
    }
    return p;
}

CodePudding user response：

I would like to know why if a local pointer with the same name as the argument is returned, it is NULL everytime.

You weren't returning a local variable.

You created a local variable, set it to allocated space, and used it to set some fields.

Then its scope ended at the end of the if block. It no longer shadowed the outer variable.

The outer variable p was never modified from its original null value.

You can avoid this by not shadowing variables.