On Wed, 23 Oct 2013 15:37:42 -0400 (EDT) Dave Anderson <anderson@xxxxxxxxxx> wrote: > ----- 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, Hi Dave, > I've reviewed the code changes, and have been beating on the > patch, and I can't get it break. Really nice work... > >[...] > > 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]] Yes! It shows that cpuspec cannot be specified for arbitrary expressions. In fact, I thought about implementing that feature, but I doubt I could get it to work without patching gdb itself. >[...] > And maybe a minor indenting change for the cpu/address values, > from this: > struct desc_ptr { > crash> struct desc_ptr b0c8:1,3 > [1]: ffff88021e24b0c8 > struct desc_ptr { >[...] > to this: > > crash> struct desc_ptr b0c8:1,3 > [1]: ffff88021e24b0c8 > struct desc_ptr { I copied this code verbatim from the "p" command, so the extra spaces are forgotten rather than intended. >[...] > But other than that, it looks good to go. Do you have anything > more to add? No, it looks good now. Should I refresh the patch set, or can you make the cosmetic changes in your tree yourself? Petr Tesarik -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility