Re: [systemd-devel] [PATCH] util: Fix signedness error in lines(), match implementations

[Lennart Poettering]

On Thu, 01.01.15 15:05, Colin Walters (walt...@verbum.org) wrote:

> From a74befe02b8a8141a2ffc5613372ef8082a2c6d2 Mon Sep 17 00:00:00 2001
> From: Colin Walters 
> Date: Thu, 1 Jan 2015 14:57:08 -0500
> Subject: [PATCH] util: Fix signedness error in lines(), match implementations
> 
> Regression introduced by ed757c0cb03eef50e8d9aeb4682401c3e9486f0b
> 
> Mirror the implementation of columns(), since the fd_columns()
> functions returns a negative integer for errors.
> 
> Also fix columns() to return the unsigned variable instead of the
> signed intermediary (they're the same, but better to be explicit).

Indeed! This actually fixes a real bug. Applied.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] util: Fix signedness error in lines(), match implementations

[Colin Walters]

On Thu, Jan 1, 2015, at 03:21 PM, Zbigniew Jędrzejewski-Szmek wrote:

> Is there an actual regression? Afaict, your patch does not change
> the behaviour in any way...

I can't think of an actual real world regression.  I could have said
"error introduced by" or something?  It only fixes the case where
the ioctl(fd, TIOCGWINSZ, &ws) fails for some reason - we would
end up thinking the screen had some very large number of lines.

(I only noticed this as I was copying the code)

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] util: Fix signedness error in lines(), match implementations

[Zbigniew Jędrzejewski-Szmek]

On Thu, Jan 01, 2015 at 03:05:15PM -0500, Colin Walters wrote:
> 

> From a74befe02b8a8141a2ffc5613372ef8082a2c6d2 Mon Sep 17 00:00:00 2001
> From: Colin Walters 
> Date: Thu, 1 Jan 2015 14:57:08 -0500
> Subject: [PATCH] util: Fix signedness error in lines(), match implementations
> 
> Regression introduced by ed757c0cb03eef50e8d9aeb4682401c3e9486f0b
Is there an actual regression? Afaict, your patch does not change
the behaviour in any way...

Zbyszek

> Mirror the implementation of columns(), since the fd_columns()
> functions returns a negative integer for errors.
> 
> Also fix columns() to return the unsigned variable instead of the
> signed intermediary (they're the same, but better to be explicit).
> ---
>  src/shared/util.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/shared/util.c b/src/shared/util.c
> index dfaf7f7..390ad1d 100644
> --- a/src/shared/util.c
> +++ b/src/shared/util.c
> @@ -3456,7 +3456,7 @@ unsigned columns(void) {
>  c = 80;
>  
>  cached_columns = c;
> -return c;
> +return cached_columns;
>  }
>  
>  int fd_lines(int fd) {
> @@ -3473,7 +3473,7 @@ int fd_lines(int fd) {
>  
>  unsigned lines(void) {
>  const char *e;
> -unsigned l;
> +int l;
>  
>  if (_likely_(cached_lines > 0))
>  return cached_lines;
> @@ -3481,7 +3481,7 @@ unsigned lines(void) {
>  l = 0;
>  e = getenv("LINES");
>  if (e)
> -(void) safe_atou(e, &l);
> +(void) safe_atoi(e, &l);
>  
>  if (l <= 0)
>  l = fd_lines(STDOUT_FILENO);
> -- 
> 1.8.3.1
> 

> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] util: Fix signedness error in lines(), match implementations

[Colin Walters]

From a74befe02b8a8141a2ffc5613372ef8082a2c6d2 Mon Sep 17 00:00:00 2001
From: Colin Walters 
Date: Thu, 1 Jan 2015 14:57:08 -0500
Subject: [PATCH] util: Fix signedness error in lines(), match implementations

Regression introduced by ed757c0cb03eef50e8d9aeb4682401c3e9486f0b

Mirror the implementation of columns(), since the fd_columns()
functions returns a negative integer for errors.

Also fix columns() to return the unsigned variable instead of the
signed intermediary (they're the same, but better to be explicit).
---
 src/shared/util.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/shared/util.c b/src/shared/util.c
index dfaf7f7..390ad1d 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -3456,7 +3456,7 @@ unsigned columns(void) {
 c = 80;
 
 cached_columns = c;
-return c;
+return cached_columns;
 }
 
 int fd_lines(int fd) {
@@ -3473,7 +3473,7 @@ int fd_lines(int fd) {
 
 unsigned lines(void) {
 const char *e;
-unsigned l;
+int l;
 
 if (_likely_(cached_lines > 0))
 return cached_lines;
@@ -3481,7 +3481,7 @@ unsigned lines(void) {
 l = 0;
 e = getenv("LINES");
 if (e)
-(void) safe_atou(e, &l);
+(void) safe_atoi(e, &l);
 
 if (l <= 0)
 l = fd_lines(STDOUT_FILENO);
-- 
1.8.3.1

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel