----- 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 -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility