Em Thu, Apr 13, 2017 at 10:40:50AM -0500, Paul Clarke escreveu:
> On 04/12/2017 09:20 PM, Masami Hiramatsu wrote:
> > On Wed, 12 Apr 2017 09:41:51 -0500
> > Paul Clarke <p...@us.ibm.com> wrote:
> > > static struct symbol *symbols__find_by_name(struct rb_root *symbols,
> > > - const char *name)
> > > + const char *name,
> > > + unsigned int includes)
> >
> > Here, you might miss replacing this 'unsigned int' with enum.
> > (actually, enum is equal to int, not unsigned int)
>
> (Ugh.) My bad. Will fix.
>
> > > +enum symbols_tag_includes {
> > > + SYMBOLS_TAG__INCLUDE_NONE,
> > > + SYMBOLS_TAG__INCLUDE_DEFAULT_ONLY
> > > +};
> >
> > BTW, would we need such 's' for plural and third person singular for type
> > name?
> > And also, you should use enum type name for prefix so that other developers
> > easily find the definition of enumeration, e.g.
> >
> > enum symbol_tag_include {
> > SYMBOL_TAG_INCLUDE__NONE = 0,
> > SYMBOL_TAG_INCLUDE__DEFAULT_ONLY
> > };
> I was thinking the top-level namespace would be "symbols", because we
yeah, we have both namespaces: symbols__ and symbol__, the first for
operations on rbtrees of the later, for instance, from symbol.h:
void symbol__delete(struct symbol *sym);
void symbols__delete(struct rb_root *symbols);
symbols__delete() will call symbol__delete() for each entry in that
rbtree.
> are not necessarily working with a single symbol. Secondary namespace
> would be "tag", since this enum is very specific to tags. Then, the
> actions are whether to "include none (of tagged symbols)" or "include
> only symbols tagged as default".
> I'm fine with your suggestion, though, and will submit a new patch
> incorporating that soon.
>
> Regards,
> PC
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
