RE: [PATCH] tools/gputop: Fix zero output when stdout is not a terminal

Srinivas, Vidya
2026-04-09T16:59:01Z

> -----Original Message-----
> From: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Sent: 09 April 2026 18:53
> To: Srinivas, Vidya <vidya.srinivas@intel.com>
> Cc: igt-dev@lists.freedesktop.org; Purkait, Soham
> <soham.purkait@intel.com>
> Subject: Re: [PATCH] tools/gputop: Fix zero output when stdout is not a
> terminal
>
> Hi Vidya,
> On 2026-04-07 at 19:09:40 +0530, Vidya Srinivas wrote:
> > When gputop output is redirected to a file or run via adb shell
>
> I checked and
>
> ./gputop -n10 -d0.1 > out
>
> works perfectly fine.
>
> > (e.g., on Android), update_console_size() calls ioctl(0, TIOCGWINSZ)
> > on stdin which may not be a terminal. If stdin is a pipe or
> > redirected, the ioctl returns -1 and con_w/con_h remain at their
> > initial value of -1.
>
> Adding few more nits to what Soham wrote, it could be shortened a little here
> to something like:
>
> When gputop is run via adb shell on Android, it could have no terminal, so
> calls ioctl(0, TIOCGWINSZ) will return -1 and function update_console_size()
> will not fill con_w/con_h.
>
> >
> > This causes the main display loop to immediately break since lines >=
> > con_h evaluates to true (0 >= -1), resulting in zero output being
> > produced.
> >
> > Fix this by initializing con_w and con_h to default values of
> > 80 and 50 respectively, so that when ioctl fails the display loop
> > still produces output with sensible dimensions.
> >
> > v2: Try stdout, stdin, and stderr for terminal size before falling
> > back to defaults (Soham Purkait)
>
> Please add to Cc developer which did a review.
>
> >
> > v3: Initialize defaults in main() instead of adding fallback logic
> > in update_console_size() (Soham Purkait)
> >
>
> So here:
>
> Cc: Soham Purkait <soham.purkait@intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> > Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>
> You could keep my r-b, so this will be:
>
> Cc: Soham Purkait <soham.purkait@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
>
> Please keep only one r-b, also imho when you change your code from version
> 1, when you got r-b, to another version, and that new one did major changes
> then imho you should remove r-b from your patch. Here it was small change
> and rather trivial but please keep that in mind for future development.
>
> Imho you do not need to send new version, please just discuss with Soham
> and agree on final description.

Thank you Kamil. Apologies for the misses.
Will take care next time.

Regards
Vidya
>
> Regards,
> Kamil
>
> > ---
> > tools/gputop.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/gputop.c b/tools/gputop.c index
> > 9b2e8cb6f..0ed5af456 100644
> > --- a/tools/gputop.c
> > +++ b/tools/gputop.c
> > @@ -609,7 +609,7 @@ int main(int argc, char **argv)
> > unsigned int period_us;
> > struct igt_profiled_device *profiled_devices = NULL;
> > struct igt_drm_clients *clients = NULL;
> > - int con_w = -1, con_h = -1;
> > + int con_w = 80, con_h = 50;
> > int ret;
> > long n;
> >
> > --
> > 2.45.2
> >