[Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully
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
Reviewed-by: Kamil Konieczny
---
tools/intel_gpu_top.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 006879c4ae67..00506c63db4e 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -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
Re: [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully
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
Re: [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully
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.
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
Re: [Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully
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?
> ---
> 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?
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
>
[Intel-gfx] [PATCH i-g-t 4/4] tools/intel_gpu_top: Handle narrow terminals more gracefully
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
---
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)
{
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
