Re: [PATCH v2 04/47] perf bench: Silence -Wshorten-64-to-32 warnings

From: Ian Rogers
Date: Wed Apr 30 2025 - 18:23:06 EST


On Wed, Apr 30, 2025 at 3:19 PM Dirk Gouders <dirk@xxxxxxxxxxx> wrote:
>
> Ian Rogers <irogers@xxxxxxxxxx> writes:
>
> > On Wed, Apr 30, 2025 at 1:23 PM Dirk Gouders <dirk@xxxxxxxxxxx> wrote:
> >>
> >> Hi Ian,
> >>
> >> considering so many eyes looking at this, I am probably wrong.
> >>
> >> So, this is only a "gauge reply" to see if it's worth I really read
> >> through all the commits ;-)
> >>
> >> Ian Rogers <irogers@xxxxxxxxxx> writes:
> >>
> >> [SNIP]
> >>
> >> > diff --git a/tools/perf/bench/sched-pipe.c b/tools/perf/bench/sched-pipe.c
> >> > index 70139036d68f..b847213fd616 100644
> >> > --- a/tools/perf/bench/sched-pipe.c
> >> > +++ b/tools/perf/bench/sched-pipe.c
> >> > @@ -102,7 +102,8 @@ static const char * const bench_sched_pipe_usage[] = {
> >> > static int enter_cgroup(int nr)
> >> > {
> >> > char buf[32];
> >> > - int fd, len, ret;
> >> > + int fd;
> >> > + ssize_t ret, len;
> >> > int saved_errno;
> >> > struct cgroup *cgrp;
> >> > pid_t pid;
> >> > @@ -118,7 +119,7 @@ static int enter_cgroup(int nr)
> >> > cgrp = cgrps[nr];
> >> >
> >> > if (threaded)
> >> > - pid = syscall(__NR_gettid);
> >> > + pid = (pid_t)syscall(__NR_gettid);
> >> > else
> >> > pid = getpid();
> >> >
> >> > @@ -172,23 +173,25 @@ static void exit_cgroup(int nr)
> >> >
> >> > static inline int read_pipe(struct thread_data *td)
> >> > {
> >> > - int ret, m;
> >> > + ssize_t ret;
> >> > + int m;
> >> > retry:
> >> > if (nonblocking) {
> >> > ret = epoll_wait(td->epoll_fd, &td->epoll_ev, 1, -1);
> >>
> >> The epoll_wait(), I know of, returns an int and not ssize_t.
> >>
> >> That shouldn't show up, because it doesn't cause real problems...
> >
> > So the function is read_pipe so it should probably return a ssize_t. I
> > stopped short of that but made ret a ssize_t to silence the truncation
> > warning on the read call. Assigning smaller to bigger is of course not
> > an issue for epoll_wait.
>
> Oh yes, I missed that ret is also used for the result of read().
>
> Some lines down there is also a combination of
>
> ret = enter_cgroup() (which is int)
>
> and
>
> ret = write()
>
>
> Just confusing but yes, because ret is also used for read() and write()
> in those cases it should be ssize_t.
>
> I'm sorry for the noise.

No worries, I'm appreciative of the eyes. I suspect we'll only pick up
the first patches in this series to fix what is a bug on ARM. I think
I'm responsible for too much noise here ;-)

Thanks,
Ian

> Best regards,
>
> Dirk