Re: [PATCH] perf tools: Don't write to evsel if parser doesn't collect evsel

From: Arnaldo Carvalho de Melo
Date: Wed Sep 02 2015 - 09:55:25 EST


Em Wed, Sep 02, 2015 at 08:05:54PM +0800, pi3orama escreveu:
> åèæç iPhone
> > å 2015å9æ2æïäå7:54ïJiri Olsa <jolsa@xxxxxxxxxx> åéï
> >> On Wed, Sep 02, 2015 at 02:53:58PM +0800, Wangnan (F) wrote:
> >>> @@ -1252,7 +1262,13 @@ foreach_evsel_in_last_glob(struct perf_evlist *evlist,
> >>> struct perf_evsel *last = NULL;
> >>> int err;
> >>> - if (evlist->nr_entries > 0)
> >>> + /*
> >>> + * Don't return when list_empty, give func a chance to report
> >>> + * error when it found last == NULL.
> >>> + *
> >>> + * So no need to WARN here, let *func do this.
> >>> + */
> >>> + if (!list_empty(&evlist->entries))

> > why is it better than to check evlist->nr_entries?
> > evlist->nr_entries is equivalent to !list_empty(&evlist->entries) in here, right?

> By checking list we won't rely on the assumption that nr_entries reflects the
> actual number of elements in that list, makes the logic of this code more compact.

But why would we want to break that assumption?

If I see FOO->entries and FOO->nr_entries, it is reasonable to expect
that whatever data structure FOO->entries may be has FOO->nr_entries in
it, lets not break that assumption.

- Arnaldo

> Don't you think so?
>
> At this point they are equivalent, but the whole patch is preventive action.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/