[Crash-utility] [PATCH] dis: Consolidate cmd_dis

Dave Anderson anderson at redhat.com
Wed Aug 5 21:04:37 UTC 2015



----- Original Message -----





 > > On Tue 2015-08-04 14:20 -0400, Dave Anderson wrote:
> > > > > Hi Dave,
> > > > > 
> > > > > No functionality change (hopefully). This patch is essentially a clean up
> > > > > in preparation to introduce the "-f" option.
> > > > > 
> > > > > Please let me know your thoughts.
> > > > 
> > > > Honestly, I've looked at the patch both in text mode and in a graphical
> > > > diff, and I have to say, it's really difficult to understand exactly what
> > > > the patch accomplishes. Things seem to work as expected AFAICT.
> > > 
> > > I wanted to remove most of code duplication and somewhat reduce the
> > > complexity of the function. This is the sole purpose of this patch.
> > > With regards to the "-f" option feature, I would prefer to build on this
> > > version.
> > > 
> > > --
> > > Aaron Tomlin
> > 
> > OK.  I did notice that it managed to cut ~70 lines.  I'll do a side-by-side 
> > comparison sanity check and continue testing it.
> 
> Thanks Dave.
> 
> > 
> > Anyway, the -f implementation on top of it should be fairly trivial, correct?
> > It should essentially be the same as GNU_FUNCTION_ONLY, but with an additional
> > "continue" in the lower while loop until curaddr reaches the starting address,
> > right?  
> 
> Yes -- until curaddr matches the given target address then proceed.
> 
> -- 
> Aaron Tomlin

OK, I see what you've done, and "Consolidate" makes perfect sense to me now. 
 
While you're at it, the crufty "#ifdef OLDWAY" section should be removed as well,
since it's just a waste of space.

Since the -f implementation should be simple, do you have a problem with
posting a single patchset that does both the consolidation and the new 
functionality (and thereby would have a single changelog entry)?

Thanks,
  Dave








More information about the Crash-utility mailing list