-3

the code is in order to create a binary tree stored in a sequential structure, and to be able to traverse the binary tree in a previous order. However , when I create the binary tree, it can't output. Why is there a problem with the creation of binary trees? Is the struct has something wrong? please tell me how can I solve this problem? I will be appreciate you for what you have done.

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

typedef struct node
{
    int data;
    struct node* left;
    struct node* right;
}Tree;
typedef struct bit
{
    Tree *a[100];
    int length;
}Bitree;

typedef struct Stack
{
    Tree *sq[1000];
    int top;
}stack;

int empty(stack s)
{
    return s.top==-1;
}

void push(stack *s,Tree *p)
{
    s->sq[++s->top]=p;
}

void pop(stack *s)
{
    if(s->top!=-1)
    {
        s->top--;
    }
}
//return the top element


Tree *top(stack s)
{
    if(s.top!=-1)
        return s.sq[s.top];
}
Bitree *create(Bitree *tree1,int n)
{
    int x;
    tree1->a[0]->data=1;
    printf("%d ",tree1->a[0]->data);
    tree1->length=0;
    printf("请输入根节点\n");
    scanf("%d ",&x);
    tree1->a[1]->data=x;
    tree1->length++;
    for(int i=2;i<=n;i++)
    {
        if(i%2==0)
        {
            printf("please input left binary tree\n");
            scanf("%d ",&x);
            tree1->a[i]->data=x;
            tree1->a[i/2]->left=tree1->a[i];
            tree1->length++;
        }
        else
        {
            printf("please input right binary tree\n");
            scanf("%d ",&x);
            tree1->a[i]->data=x;
            tree1->a[i/2]->right=tree1->a[i];
            tree1->length++;
        }
    }
    return tree1;
}
void preorder1(Bitree *t)
{
    stack s;
    s.top=-1;
    if(t->a[1]!=NULL) {
        push(&s,t->a[1]);
    }
    while(!empty(s))
    {
        Tree *x=top(s);
        pop(&s);
        printf("%d ",x->data);
        if(x->right!=NULL)
            push(&s,x->right);
        if(x->left!=NULL)
            push(&s,x->left);
    }
}
int main()
{

    int n;
    Bitree *t1;
    scanf("%d",&n);
    t1=create(t1,n);
    preorder1(t1);
}
waggghhh
  • 1
  • 1
  • Have you tried running your code line by line in a debugger while monitoring the values of all variables, in order to determine at which point your program stops behaving as intended? If you did not try this, then you may want to read this: [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/q/25385173/12149471) – Andreas Wenzel Oct 24 '21 at 12:58
  • 1
    Please provide a [mre] with exact input, intended output and actual output. The statement "it can't output" is not a sufficient description of the error. Does the program not output anything although you expect it to? Or is it outputting what seems to be random characters? – Andreas Wenzel Oct 24 '21 at 13:02
  • 2
    `printf("请输入根节点\n");` risks undefined behavior as the encoding may contain a byte of `%` leading to trouble. Better to use `printf("%s", "请输入根节点\n");` or `fwprint()`. – chux - Reinstate Monica Oct 24 '21 at 13:20
  • `s->sq[++s->top]=p` is undefined behavior. If an object is modified, it's only permitted to access its value for the purpose of modification. `s->sq` violates that. – sj95126 Oct 24 '21 at 13:21
  • No, @sj95126, that applies only to *scalar* objects. Neither `*s` not `s->sq` is a scalar. – John Bollinger Oct 24 '21 at 13:24
  • @JohnBollinger: `s` is the scalar object. It's being modified (`++s`) and accessed (dereferenced as `s->sq`). – sj95126 Oct 24 '21 at 13:24
  • @sj95126, `++s` is not evaluated in that expression. The `->` has higher precedence than prefix `++`, so the expression is equivalent to `s->sq[++(s->top)]=p`. – John Bollinger Oct 24 '21 at 13:26
  • 1
    @JohnBollinger: sorry, my mistake. It's *postfix* increment that has the same precedence as `->` – sj95126 Oct 24 '21 at 13:28

2 Answers2

1

First problem I see , you forgot to allocate your Bitree and the a structure in it . try this :

int main()
{

int n;
Bitree *t1 = (Bitree*) malloc(sizeof(Bitree));
int i = 0 ; 
for(i ; i < 100 ; i++)
        t1->a[i] = (Tree*) malloc(sizeof(Tree)); 
scanf("%d",&n);
t1=create(t1,n);
preorder1(t1);
for(i = 0 ; i < 100 ; i++)
        free(t1->a[i]); 
free(t1); 
}
Amine Bensalem
  • 362
  • 3
  • 15
  • thanks your answer. but your answer may be wrong. First, t1 , t1->a[i] have space and shouldn't be malloc . Second , I use your main function to test ,it still has something wrong. – waggghhh Oct 24 '21 at 12:27
  • @waggghhh oh , it has already been allocated then ? My bad then , I thought you had an issue with the segfault . So , your issue is more with the algorithm used , I guess ? – Amine Bensalem Oct 24 '21 at 12:44
  • yes, my issue is about why the algorithm used in the create binary tree in a sequential structure. when I use the create function , it even can't printf my tips and can't create t1 correctly. – waggghhh Oct 24 '21 at 12:53
  • @waggghhh, the code presented in the question *does not* allocate space for `main()`'s variable `t1` to point to, neither in `main` nor in `create`, yet `create()` assumes that the pointer it receives as its argument is valid. That is definitely an error, just as this answer points out. If `t1` did have sufficient space allocated then that would include storage for the 100 elements of `t1->a`, but that's a separate matter. – John Bollinger Oct 24 '21 at 12:55
  • @waggghhh are you sure you correctly allocated your structures like I did ? Because I definitely have your printfs and scanfs working on my setup. I didn't verify if your preorder function is correct on the algorithmic aspect yet , but if you don't have anything printed out , it's surely because your structures are not allocated. – Amine Bensalem Oct 24 '21 at 12:58
  • @waggghhh, Also, the the elements of `t1->a` would exist if `*t1` in fact existed, but these are *pointers* that, just like `t1` in actuality, don't point to anything unless you assign valid pointer values to them. So this answer is exactly right about both of those issues (+1). It is possible to resolve those issues without dynamic memory allocation, but these together constitute a large part of your problem. – John Bollinger Oct 24 '21 at 15:05
0

Major issues with this program include:

  • Dereferencing the indeterminate initial value of main()'s pointer t1, as @AsmineBensalem observed first.

    The signature of function create() suggests that there may have been some confusion about how space was supposed to be allocated -- create() receives a pointer to a Bitree, but also returns one. If that function were expected to allocate the space, then it would not need to receive a pointer to existing space, but given that it in fact relies on the caller to provide a valid Bitree *, there seems little point in it echoing that same pointer value back to the caller.

    For the record, given create() as presently written, I would write main() like so (without malloc()):

    int main(void) {
        int n;
        Bitree t1 = { .length = 0 };  // not a pointer; with initializer
    
        scanf("%d", &n);
        create(&t1, n);
        preorder1(&t1);
    }
    

    Declaring t1 as a Bitree instead of a Bitree * provides storage for the whole Bitree instead of just for a pointer to one. See below for more about the initializer.

  • Dereferencing the values of elements of t1->a when these (pointer) values are indeterminate, also as @AsmineBensalem observed first. This is essentially the same as the previous issue, and its recurrence suggests that you do not understand that declaring a pointer gets you only a pointer, not an object for it to point to. Since you seem to be trying to avoid dynamic allocation, you could start a solution to this by changing the definition of type Bitree to contain an array of the needed Tree objects instead of pointers to such objects:

    typedef struct bit {
        Tree a[100];  // array of Tree, not of Tree *
        int length;
    } Bitree;
    

    That will require several additional changes elsewhere in the code, but these are straightforward.

  • Failing to initialize the left and right pointers of nodes that have fewer than two children. Your preorder() function assumes that these will be NULL where a node has no child in the specified direction, but create() does not ensure that.

    You could, however, get that automatically by changing the definition of Bitree as described in the previous point, and ensuring that your Bitree object is declared with an initializer, as shown in the first point above. In that case, you don't have to explicitly initialize all the pointers to get null initial values for them, but you do need to provide an initializer for at least some part of the Bitree.

    Alternatively, you could just have create() explicitly assign NULL to the child pointers of each node it initializes.

Additional issues include:

  • Function create()'s scanf() formats cause misleading behavior. This code appears three times:

        scanf("%d ",&x);
    

    The trailing whitespace in the format matches any amount of whitespace, so scanf has to match it by continuing to read whitespace until it sees non-witespace. The practical effect is that the user has to input the value for each non-root node before the prompt for that node is printed, and they need to input an extra value or some kind of trailing junk in order to complete the input stage. Just remove the space at the end of each of these formats:

        scanf("%d", &x);
    
  • Function top() is declared to return a Tree *, but when the stack passed to it is empty, it terminates without returning anything.

  • Functions top() and empty() each receive a stack as an argument, by value. Although this is not inherently wrong, it does, in principle, involve making a copy of the caller's argument at every call. stack is a pretty big structure, so this is undesirable.

  • It is surprising that pop() pops an element from the stack without returning it. You work with that by first retrieving the top element with top() and then pop()ping it, but it would be much more conventional to have pop() both pop an element off the stack and return that element.

  • It is surprising and unnecessary that create() does not use the tree node at index 0. (I discount entering a dummy value into it, since that's purposeless when you don't mean for that value to be accessed elsewhere.)

John Bollinger
  • 160,171
  • 8
  • 81
  • 157