Re: [Crash-utility] Adding a new command rbtree
----- 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