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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 2 Sep 2015 22:04:21 +0800
From:	pi3orama <pi3orama@....com>
To:	Arnaldo Carvalho de Melo <acme@...hat.com>
Cc:	Jiri Olsa <jolsa@...hat.com>, "Wangnan (F)" <wangnan0@...wei.com>,
	"masami.hiramatsu.pt@...achi.com" <masami.hiramatsu.pt@...achi.com>,
	Alexei Starovoitov <ast@...mgrid.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Namhyung Kim <namhyung@...nel.org>,
	Zefan Li <lizefan@...wei.com>,
	"acme@...nel.org" <acme@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] perf tools: Don't write to evsel if parser doesn't collect evsel



发自我的 iPhone

> 在 2015年9月2日，下午9:55，Arnaldo Carvalho de Melo <acme@...hat.com> 写道：
> 
> Em Wed, Sep 02, 2015 at 08:05:54PM +0800, pi3orama escreveu:
>> 发自我的 iPhone
>>> 在 2015年9月2日，下午7:54，Jiri Olsa <jolsa@...hat.com> 写道：
>>>>> 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.

Then we should enforce it. For example, check the list collected by parser,
report an error if the list is empty, to avoid someone like me adding
nothing on the list but report success. I'm not insistent on this patch. In my newest
patch set I use real dummy evsel as placeholder so we won't meet empty list again.

Thank you.

> 
> - 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/