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]

[Bug tapsets/10466] stap -L misrepresents variables available to multi-probe aliases


------- Additional Comments From jistone at redhat dot com  2009-11-03 19:42 -------
(In reply to comment #3)
> Created an attachment (id=4353)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=4353&action=view)
> Updated patch

Great, that's exactly the output I was hoping for!  I have some comments for
cleaning up the code, and then please go ahead and commit this.

> map<string,set<unsigned>&> probe_list;

The reference value here seems dubious, and I also wonder why not just keep the
probe pointer directly instead of indexes?

  map<string, set<derived_probe*> > probe_list;

You can also take advantage that the default set constructor is an empty set, so
this:

> if (probe_list.find (pp) == probe_list.end())
>   {
>     set<unsigned> *probe_id = new set<unsigned>();
>     (*probe_id).insert(i);
>     probe_list.insert(pair<string,set<unsigned>&>(pp,*probe_id));
>   }
> else
>   probe_list.find(pp)->second.insert(i);

becomes:

  probe_list[pp].insert(p);

This same idiom will work for updating the counts in var_list and arg_list.

  var_list[tmps.str()]++;
...
  arg_list[*ia]++;

> set<string> *arg_set = new set<string>();
> p->getargs(*arg_set);

There's no reason for this to be heap allocated, which you're then leaking. 
Just create it directly:

  set<string> arg_set;
  p->getargs(arg_set);

>         var_list.clear();
>         arg_list.clear();
>       }
>     o << endl;
>   }
> probe_list.clear();

These clear() calls are redundant, as the maps are already going out of scope
there and will be deconstructed.

> dwarf_derived_probe::getargs(std::set<std::string> &arg_set) const
> {
>   for (set<string>::iterator it = args.begin(); it != args.end(); it++)
>     arg_set.insert(*it);
> }

Note that set::insert can take iterators:

  arg_set.insert(args.begin(), args.end());


Thanks!


-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=10466

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.


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