This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: loc2c-test with dwarf_entry_breakpoints


On Fri, Feb 22, 2013 at 05:02:09PM -0500, Lukas Berk wrote:
> Hey Mark,
> 
> Thanks for the review, 
> 
> > The loc2c-test.patch.1 doesn't seem to implement the @functionname
> > variant (as in select the entry-breakpoint address), but the plain
> > functionname variant works as expected. nice.
> 
> Ah, after clarification I've fixed it in the patch attached.

Thanks, works nicely to see the difference in location expression
at the start/entry address for a variable.

> > - The '\t' in the output when listing all functions with their entry
> >   and breakpoint addresses make things very, very wide.
> >   Could you consider just using spaces?
> 
> I've adjusted the printf's to remove the \t's and generally the output
> should be less than 80 chars (unless the function name is really long).

One more little nitpick, the "header" should show the same spacing.
Something like:

      printf ("Function                             "
              "Address            Debug Entry Address(s)\n");

OK, and another, matchdebug should be bool, not int.

> The patch attached combines the changes, let me know if you'd like it
> separated again for clarity.

No it looks fine as is. With one more suggestion below.

> +      a.funcname = argv[argi];
> +      char* at = strchr (a.funcname, '@');
> +      /* We need to check and adjust for a possible '@' character */
> +      if(at != NULL)
> +	{
> +	  *at++ = '\0';
> +	  a.funcname = at;

This would be simpler as follows:

      a.funcname = argv[argi];
      /* We need to check and adjust for a possible '@' character */
      if(a.funcname[0] == '@')
        {
          a.funcname++;

Thanks,

Mark


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]