Re: tools/gputop: Fix zero output when stdout is not a terminal

Purkait, Soham
2026-04-06T16:34:58Z

[-- Attachment #1: Type: text/plain, Size: 3388 bytes --]

Hi Vidya,

On 06-04-2026 11:11, Srinivas, Vidya wrote:
>
>> -----Original Message-----
>> From: Purkait, Soham<soham.purkait@intel.com>
>> Sent: 06 April 2026 10:27
>> To: Srinivas, Vidya<vidya.srinivas@intel.com>;igt-dev@lists.freedesktop.org;
>> Konieczny, Kamil<kamil.konieczny@intel.com>
>> Subject: Re: tools/gputop: Fix zero output when stdout is not a terminal
>>
>> Hi Vidya,
>>
>> On 02-04-2026 19:15, Vidya Srinivas wrote:
>>> When gputop output is redirected to a file or pipe, such as:
>>>
>>> gputop -n 5 -d 1 > results.txt
>> Isn’t only stdout (fd 1) redirected to the file here?
>>
>> I guess some thing like "echo test | ./gputop" would be returning the default
>> value (-1 in this case) for con_h.
> Hello,
>
> Many thanks for the review.
> I will update the patch like this. Should be okay?
>
> if (ioctl(STDOUT_FILENO, TIOCGWINSZ, &ws) == -1 &&
> ioctl(STDIN_FILENO, TIOCGWINSZ, &ws) == -1 &&
> ioctl(STDERR_FILENO, TIOCGWINSZ, &ws) == -1) {

I assume it's better to keep this : "if (ioctl(0, TIOCGWINSZ, &ws) ==
-1)" as is. As it won't make any  issue if the gputop output is
redirected to any file or so (which only involve stdout).  The
expression  will only be true when stdin is not a terminal, may be
during running gputop in a ci.
And Its better to assign the default values to con_w and con_h in main.
(please see below)

> ws.ws_col = 80;
> ws.ws_row = 50;
> }
>
> Regards
> Vidya
>
>>> gputop -n 3 -d 2 | grep rcs
>>>
>>> ioctl(0, TIOCGWINSZ) fails with -1 since stdin is not a terminal.
>>> update_console_size() then returns without setting *w and *h, leaving
>>> con_w and con_h at their initial value of -1.
>>>
>>> The main display loop uses 'if (lines >= con_h) break' to limit output
>>> to the terminal height. With con_h = -1, the condition (0 >= -1) is
>>> immediately true on the very first line, causing all client output to
>>> be silently suppressed. The result is that gputop produces only ANSI
>>> clear- screen escape sequences and zero actual data.
>>>
>>> This affects anyone using gputop in automation, CI pipelines, or any
>>> non-interactive context on Linux or Android where output is redirected
>>> or piped.
>>>
>>> Fix this by falling back to a default console size of 80x50 when the
>>> ioctl fails, consistent with the existing fallback for serial consoles
>>> (where ws_col and ws_row are both 0).
>>>
>>> Signed-off-by: Vidya Srinivas<vidya.srinivas@intel.com>
>>> Reviewed-by: Kamil Konieczny<kamil.konieczny@linux.intel.com>
>>> ---
>>> tools/gputop.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/gputop.c b/tools/gputop.c index
>>> 9b2e8cb6f..112ec5ddb 100644
>>> --- a/tools/gputop.c
>>> +++ b/tools/gputop.c
>>> @@ -505,8 +505,11 @@ static void update_console_size(int *w, int *h)
>>> {
>>> struct winsize ws = {};
>>>
>>> - if (ioctl(0, TIOCGWINSZ, &ws) == -1)
>>> + if (ioctl(0, TIOCGWINSZ, &ws) == -1) {
>>> + *w = 80;
>>> + *h = 50;

Imho instead of setting the values (*w = 80;*h = 50) here , it's better
to initialize these as default values in main as "int con_w = 80, con_h
= 50;"

Thanks, Soham

>> How about assigning the values during initialization in the main function ?
>>
>> eg : int con_w = 80, con_h = 50;
>>
>> Thanks,
>> Soham
>>
>>> return;
>>> + }
>>>
>>> *w = ws.ws_col;
>>> *h = ws.ws_row;

[-- Attachment #2: Type: text/html, Size: 5456 bytes --]