From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754618AbbIBMqV (ORCPT ); Wed, 2 Sep 2015 08:46:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40640 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753106AbbIBMqU (ORCPT ); Wed, 2 Sep 2015 08:46:20 -0400 Date: Wed, 2 Sep 2015 14:46:16 +0200 From: Jiri Olsa To: pi3orama Cc: "Wangnan (F)" , "masami.hiramatsu.pt@hitachi.com" , "acme@redhat.com" , Alexei Starovoitov , Jiri Olsa , Namhyung Kim , Zefan Li , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] perf tools: Don't write to evsel if parser doesn't collect evsel Message-ID: <20150902124616.GI8598@krava.brq.redhat.com> References: <50399556C9727B4D88A595C8584AAB37525027C2@GSjpTKYDCembx32.service.hitachi.net> <1441176553-116129-1-git-send-email-wangnan0@huawei.com> <55E69D06.4050005@huawei.com> <20150902115415.GH8598@krava.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 02, 2015 at 08:05:54PM +0800, pi3orama wrote: SNIP > >>> perf_evlist__splice_list_tail(evlist, &data.list, entries); > >>> evlist->nr_groups += data.nr_groups; > >>> - last = perf_evlist__last(evlist); > >>> - last->cmdline_group_boundary = true; > >>> return 0; > >>> } > >>> @@ -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. > Don't you think so? > > At this point they are equivalent, but the whole patch is preventive action. ok, fair enough ;-) Acked-by: Jiri Olsa thanks, jirka