Re: [Question] About '(*idx)++' of perf_evsel__new_idx

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Mon, Jan 30, 2017 at 11:53:18AM +0900, Taeung Song escreveu:
> Hi, :)
> 
> Can I ask you one thing ?
> 
> I'm reading source code util/parse-event.c
> Along the way, I wonder why increase idx before checking
> whether 'evsel' is NULL or not ? (at 310,311 line number)
> 
> 
> 300 static struct perf_evsel *
> 301 __add_event(struct list_head *list, int *idx,
> 302             struct perf_event_attr *attr,
> 303             char *name, struct cpu_map *cpus,
> 304             struct list_head *config_terms)
> 305 {
> 306         struct perf_evsel *evsel;
> 307
> 308         event_attr_init(attr);
> 309
> 310         evsel = perf_evsel__new_idx(attr, (*idx)++);
> 311         if (!evsel)
> 312                 return NULL;
> 313
> 314         evsel->cpus     = cpu_map__get(cpus);
> 315         evsel->own_cpus = cpu_map__get(cpus);
> 316
> 
> IMHO, if 'evsel' isn't NULL, we can increase idx like below.
> 
>             evsel = perf_evsel__new_idx(attr, *idx);
>             if (!evsel)
>                     return NULL;
>             else
>                (*idx)++;
> 
> Is it wrong ? or is there other reason about increasing idx
> before check 'evsel'?

I think you're right and we should increment idx only if we manage to
create the evsel instance, no need for the else clause tho.

- Arnaldo