Re: [PATCH v5 1/2] perf stat: Close cork_fd when create_perf_stat_counter() failed

[Yeo Reum Yun]

Hi Andi,

> Technically 0 is a valid file descriptor. Check for >= 0

Right. However, when I see the event, evlist__start_workload(),
it checks cork_fd with

    if (evlist->workload.cork_fd > 0) {

Would you like to change this with this patchset too?

Thanks.

________________________________________
From: Andi Kleen <ak@xxxxxxxxxxxxxxx>
Sent: 25 September 2024 13:15
To: Yeo Reum Yun
Cc: peterz@xxxxxxxxxxxxx; mingo@xxxxxxxxxx; acme@xxxxxxxxxx; namhyung@xxxxxxxxxx; Mark Rutland; alexander.shishkin@xxxxxxxxxxxxxxx; jolsa@xxxxxxxxxx; irogers@xxxxxxxxxx; adrian.hunter@xxxxxxxxx; james.clark@xxxxxxxxxx; howardchu95@xxxxxxxxx; nd; linux-perf-users@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
Subject: Re: [PATCH v5 1/2] perf stat: Close cork_fd when create_perf_stat_counter() failed

Levi Yun <yeoreum.yun@xxxxxxx> writes:
> +void evlist__cancel_workload(struct evlist *evlist)
> +{
> +     int status;
> +
> +     if (evlist->workload.cork_fd > 0) {

Technically 0 is a valid file descriptor. Check for >= 0
And make sure the field is initialized with -1

The rest looks fine to me.

Reviewed-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>