----- Original Message ----- > Hi Dave, > > I'm sorry for the last submission. It seems I forgot to refresh the > patches, so it was completely bogus. Should be fixed now. I'm also > attaching my changes as one big patch to this message. > > Petr Tesarik Hi Petr, I've reviewed the code changes, and have been beating on the patch, and I can't get it break. Really nice work... The only things I'd like to change are trivial/cosmetic. For clarity's sake I'd like to change the "p" help page to separate expressions from symbols as arguments, and then enforce the fact that the :cpuspec only applies to symbol arguments. So instead of this: crash> help p NAME p - print the value of an expression SYNOPSIS p [-x|-d][-u] expression[:cpuspec] DESCRIPTION This command passes its arguments on to gdb "print" command for evaluation. expression The expression to be evaluated. cpuspec CPU specification for per-cpu variables: : CPU of the currently selected task. :a[ll] all CPUs. :#[-#][,...] CPU list(s), e.g. "1,3,5", "1-3", or "1,3,5-7,10". -x override default output format with hexadecimal format. -d override default output format with decimal format. -u the expression evaluates to a user address reference. ... I'd change it to this: crash> help p NAME p - print the value of an expression SYNOPSIS p [-x|-d][-u] [expression | symbol[:cpuspec]] DESCRIPTION This command passes its arguments on to gdb "print" command for evaluation. expression an expression to be evaluated. symbol a kernel symbol. :cpuspec when appended to a symbol with a colon, a CPU specification for per-cpu variables: : CPU of the currently selected task. :a[ll] all CPUs. :#[-#][,...] CPU list(s), e.g. "1,3,5", "1-3", or "1,3,5-7,10". -x override default output format with hexadecimal format. -d override default output format with decimal format. -u the expression evaluates to a user address reference. ... And maybe a minor indenting change for the cpu/address values, from this: crash> struct desc_ptr b0c8:1,3 [1]: ffff88021e24b0c8 struct desc_ptr { size = 0, address = 0 } [3]: ffff88021e2cb0c8 struct desc_ptr { size = 0, address = 0 } crash> to this: crash> struct desc_ptr b0c8:1,3 [1]: ffff88021e24b0c8 struct desc_ptr { size = 0, address = 0 } [3]: ffff88021e2cb0c8 struct desc_ptr { size = 0, address = 0 } crash> But other than that, it looks good to go. Do you have anything more to add? Thanks, Dave -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility