----- Original Message ----- > 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? No, I'll take it from here -- consider it queued for crash-7.0.3. And thanks again for all your work on this, it's really a significant enhancement. Dave -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility