From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <429810f6-d955-ebb7-89f0-79d822d0bd6c@linux.intel.com> Date: Wed, 11 Oct 2023 09:31:22 +0100 MIME-Version: 1.0 Content-Language: en-US From: Tvrtko Ursulin To: Kamil Konieczny , igt-dev@lists.freedesktop.org, Intel-gfx@lists.freedesktop.org, Tvrtko Ursulin References: <20231010110714.749239-1-tvrtko.ursulin@linux.intel.com> <20231010110714.749239-5-tvrtko.ursulin@linux.intel.com> <20231010164336.dikudseydggrrux7@kamilkon-desk.igk.intel.com> <7c36db01-1534-535a-c1ce-71369bc5405a@linux.intel.com> In-Reply-To: <7c36db01-1534-535a-c1ce-71369bc5405a@linux.intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On 11/10/2023 09:22, Tvrtko Ursulin wrote: > > On 10/10/2023 17:43, Kamil Konieczny wrote: >> Hi Tvrtko, >> On 2023-10-10 at 12:07:14 +0100, Tvrtko Ursulin wrote: >>> From: Tvrtko Ursulin >>> >>> Instead of asserting just skip trying to print columns when terminal is >>> too narrow. >>> >>> At the same time fix some type confusion to fix calculations going huge. >>> >>> Signed-off-by: Tvrtko Ursulin >>> Closes: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/issues/143 >> >> Did you tested this in screensaver? I mean running intel_gpu_top >> in terminal windows under X (Gnome or other) and leaving desktop >> unattanded, entering screen saver mode (possible with screen >> turned off) and then re-enabling screen? > > I tested it by resizing the terminal to crazy small dimensions and > confirmed asserts and endless printing of spaces failure modes are > fixed. Also under the screen lock. > > But no DPMS and no console screensavers. > >> >>> --- >>>   tools/intel_gpu_top.c | 12 +++++++----- >>>   1 file changed, 7 insertions(+), 5 deletions(-) >>> >>> diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c >>> index 472ce3f13ba9..6d1397cb8214 100644 >>> --- a/tools/intel_gpu_top.c >>> +++ b/tools/intel_gpu_top.c >>> @@ -926,7 +926,7 @@ static void free_display_clients(struct >>> igt_drm_clients *clients) >>>       free(clients); >>>   } >>> -static unsigned int n_spaces(const unsigned int n) >>> +static int n_spaces(const int n) >> --------- ^^^ >> Could you make it int at your first patch touching this function? > > Honestly no, can't be bothered to churn this too much. I think argument > can be made that this patch is fixing type confusion in many places so > hopefully you can accept it as is. Ah sorry, I did make it unsigned in in a previous patch.. I will respin the whole series. Regards, Tvrtko > > Regards, > > Tvrtko > >> >> With or without this suggestion, >> Reviewed-by: Kamil Konieczny >> >> Regards, >> Kamil >> >>>   { >>>       static const char *spaces[] = { >>>           " ", >>> @@ -950,7 +950,7 @@ static unsigned int n_spaces(const unsigned int n) >>>           "                   ", >>>   #define MAX_SPACES 19 >>>       }; >>> -    unsigned int i, r = n; >>> +    int i, r = n; >>>       while (r) { >>>           if (r > MAX_SPACES) >>> @@ -972,7 +972,8 @@ print_percentage_bar(double percent, double max, >>> int max_len, bool numeric) >>>       int bar_len, i, len = max_len - 2; >>>       const int w = 8; >>> -    assert(max_len > 0); >>> +    if (len < 2) /* For edge lines '|' */ >>> +        return; >>>       bar_len = ceil(w * percent * len / max); >>>       if (bar_len > w * len) >>> @@ -986,6 +987,8 @@ print_percentage_bar(double percent, double max, >>> int max_len, bool numeric) >>>           printf("%s", bars[i]); >>>       len -= (bar_len + (w - 1)) / w; >>> +    if (len < 1) >>> +        return; >>>       n_spaces(len); >>>       putchar('|'); >>> @@ -2001,8 +2004,7 @@ print_clients_header(struct igt_drm_clients >>> *clients, int lines, >>>                    4 : clients->max_name_len; /* At least "NAME" */ >>>       if (output_mode == INTERACTIVE) { >>> -        unsigned int num_active = 0; >>> -        int len; >>> +        int len, num_active = 0; >>>           if (lines++ >= con_h) >>>               return lines; >>> -- >>> 2.39.2 >>>