[Crash-utility] Adding a new command rbtree

[Dave Anderson]

----- Original Message -----
> At 2012-6-1 2:34, Dave Anderson wrote:
> > So I believe that the 1-bit should be stripped off when a user
> > inadvertently enters such an address as a -N radix_tree_node address.
> > With this change to cmd_tree():
> >
> >          if (hexadecimal_only(args[optind], 0)) {
> >                  value = htol(args[optind], FAULT_ON_ERROR, NULL);
> >                  if (IS_KVADDR(value)) {
> >                          td->start = value;
> > +                       if ((td->start&  1)&&
> > +                           (td->flags&  TREE_NODE_POINTER)&&
> > +                           (type_flag == RADIXTREE_REQUEST))
> > +                               td->start&= ~1;
> >                          goto next_arg;
> >                  }
> >          }
> >
> > it works OK regardless of the 1-bit setting:
> 
> Hello Dave,
> 
> I have to point out that you only involved the situation that user input
> a hexadecimal number. The tree command can get starting address from
> three place.
> 
> <cut>
> 
>          if ((sp = symbol_search(args[optind]))) {
>                  td->start = sp->value;
>                  goto next_arg;
>          }
> 
>          if (!IS_A_NUMBER(args[optind])) {
>                  if (can_eval(args[optind])) {
>                          value = eval(args[optind], FAULT_ON_ERROR,
>                          NULL);
>                          if (IS_KVADDR(value)) {
>                                  td->start = value;
>                                  goto next_arg;
>                          }
>                  }
>                  error(FATAL, "invalid argument: %s\n",
>                  args[optind]);
>          }
> 
>          if (hexadecimal_only(args[optind], 0)) {
>                  value = htol(args[optind], FAULT_ON_ERROR, NULL);
>                  if (IS_KVADDR(value)) {
>                          td->start = value;
>                          goto next_arg;
>                  }
>          }
> 
> <cut>

Right, there are 3 places above where the address can be entered,
but I purposely put the 1-bit check only in the hexadecimal case.

In the symbol_search() case, there would never be a symbol value
with the 1-bit set.  And for that matter, a radix tree node
would never be a symbol since radix tree nodes come from the slab
cache.

And in the !IS_A_NUMBER() case, the input of an (expression) would 
require that the user explicitly force the 1-bit to be set, which
would be nonsensical.  And for that matter, it would be highly 
unlikely that a user would ever use the (expression) construct
to enter a radix tree node address.

So for all practical purposes, only the hexadecimal_only() case
would ever be presented with the 1-bit (as a result of a cut-and-paste).

> So I prefer change like this.
> <cut>
>          if (td->flags & TREE_NODE_POINTER) {
>                  node_p = td->start;
> +
> +               if (node_p & 1)
> +                       node_p &= ~1;
> +
>                  if (VALID_MEMBER(radix_tree_node_height)) {
>                          readmem(node_p + OFFSET(radix_tree_node_height), KVADDR,
>                                  &height, sizeof(uint),
>                                  "radix_tree_node height",
>                                  FAULT_ON_ERROR);
> <cut>

That all being said, I agree that this is a cleaner place to put it, 
so I'll make the change for crash-6.0.8.

Thanks,
  Dave