netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [iproute PATCH v2 0/3] ss: netlink_show_one() clean-up, minor output fix
@ 2017-10-31 17:47 Stefano Brivio
  2017-10-31 17:47 ` [iproute PATCH v2 1/3] ss: Remove useless width specifier in process context print Stefano Brivio
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Stefano Brivio @ 2017-10-31 17:47 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Phil Sutter

Patches 1/3 and 2/3 streamline process context prints in
netlink_show_one(). Patch 3/3 fixes a display issue happening
when Netid or State columns are not displayed.

In general, the output layout of 'ss', also in the cases handled
here, still has a number of issues. I will address further issues
more structurally in a later patchset.

Stefano Brivio (3):
  ss: Remove useless width specifier in process context print
  ss: Streamline process context printing in netlink_show_one()
  ss: Fix width calculations when Netid or State columns are missing

v2: Coding style fixes in 3/3

 misc/ss.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

-- 
2.9.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [iproute PATCH v2 1/3] ss: Remove useless width specifier in process context print
  2017-10-31 17:47 [iproute PATCH v2 0/3] ss: netlink_show_one() clean-up, minor output fix Stefano Brivio
@ 2017-10-31 17:47 ` Stefano Brivio
  2017-10-31 17:47 ` [iproute PATCH v2 2/3] ss: Streamline process context printing in netlink_show_one() Stefano Brivio
  2017-10-31 17:47 ` [iproute PATCH v2 3/3] ss: Fix width calculations when Netid or State columns are missing Stefano Brivio
  2 siblings, 0 replies; 4+ messages in thread
From: Stefano Brivio @ 2017-10-31 17:47 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Phil Sutter

Both local address and service, and remote address and service
fields are already printed out in netlink_show_one() before we
start printing process context, by calling sock_addr_print()
twice.

At this point, sock_addr_print() has already forced the remote
service field to be 'serv_width' wide -- that is, 'serv_width'
width has already been consumed, before we print process
context.

Hence, it makes no sense to force the display width of process
context to be 'serv_width' wide again: previous prints have
filled up the line already. Remove the width specifier and
prefix with a space instead, to keep this consistent with fields
which are displayed after the first output line.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 misc/ss.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index dfb438f9162c..fa026eb0934b 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -3594,10 +3594,10 @@ static int netlink_show_one(struct filter *f,
 			getpidcon(pid, &pid_context);
 
 		if (pid_context != NULL) {
-			printf("proc_ctx=%-*s ", serv_width, pid_context);
+			printf(" proc_ctx=%s", pid_context);
 			free(pid_context);
 		} else {
-			printf("proc_ctx=%-*s ", serv_width, "unavailable");
+			printf(" proc_ctx=unavailable");
 		}
 	}
 
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [iproute PATCH v2 2/3] ss: Streamline process context printing in netlink_show_one()
  2017-10-31 17:47 [iproute PATCH v2 0/3] ss: netlink_show_one() clean-up, minor output fix Stefano Brivio
  2017-10-31 17:47 ` [iproute PATCH v2 1/3] ss: Remove useless width specifier in process context print Stefano Brivio
@ 2017-10-31 17:47 ` Stefano Brivio
  2017-10-31 17:47 ` [iproute PATCH v2 3/3] ss: Fix width calculations when Netid or State columns are missing Stefano Brivio
  2 siblings, 0 replies; 4+ messages in thread
From: Stefano Brivio @ 2017-10-31 17:47 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Phil Sutter

There's no need to check 'pid_context' before calling free().

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 misc/ss.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index fa026eb0934b..fb80d84122fc 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -3593,12 +3593,8 @@ static int netlink_show_one(struct filter *f,
 		else if (pid > 0)
 			getpidcon(pid, &pid_context);
 
-		if (pid_context != NULL) {
-			printf(" proc_ctx=%s", pid_context);
-			free(pid_context);
-		} else {
-			printf(" proc_ctx=unavailable");
-		}
+		printf(" proc_ctx=%s", pid_context ? : "unavailable");
+		free(pid_context);
 	}
 
 	if (show_details) {
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [iproute PATCH v2 3/3] ss: Fix width calculations when Netid or State columns are missing
  2017-10-31 17:47 [iproute PATCH v2 0/3] ss: netlink_show_one() clean-up, minor output fix Stefano Brivio
  2017-10-31 17:47 ` [iproute PATCH v2 1/3] ss: Remove useless width specifier in process context print Stefano Brivio
  2017-10-31 17:47 ` [iproute PATCH v2 2/3] ss: Streamline process context printing in netlink_show_one() Stefano Brivio
@ 2017-10-31 17:47 ` Stefano Brivio
  2 siblings, 0 replies; 4+ messages in thread
From: Stefano Brivio @ 2017-10-31 17:47 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Phil Sutter

If Netid or State columns are missing, we must not subtract one
for each of these two columns from the remaining screen width,
while distributing available space to columns. This one
character corresponding to one delimiting space has to be
subtracted only if the columns are actually printed.

Further, in the existing implementation, if the screen width is
an odd number, one additional character is added to the width of
one of the two columns.

But if both are not printed, this filling character needs to be
added somewhere else, in order to have the right spacing
allowing us to fill lines completely.

Address and port fields are printed in pairs (local and remote),
so we can't distribute the space to any of them, because it
would be doubled. Instead, print this additional space to the
right of the Send-Q column, to keep code changes to a minimum.

This is particularly visible with 'ss -f netlink -Z'. Before
this patch, with an 80 column terminal, we have:

$ ss -f netlink -Z|head -n3
Recv-Q Send-Q Local Address:Port                 Peer Address:Port
0      0            rtnl:evolution-calen/2049           *                     pr
oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0      0            rtnl:clock-applet/1944              *                     pr
oc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023

and with an 81 column terminal:

$ ss -f netlink -Z|head -n3
Recv-Q Send-Q Local Address:Port                 Peer Address:Port
0      0            rtnl:evolution-calen/2049           *                     pro
c_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0      0            rtnl:clock-applet/1944              *                     pro
c_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023

After this patch, in both cases, the output is:
$ ss -f netlink -Z|head -n3
Recv-Q Send-Q Local Address:Port                 Peer Address:Port
0      0             rtnl:evolution-calen/2049            *
 proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
0      0             rtnl:clock-applet/1944               *
 proc_ctx=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
v2: Add whitespace around '=' in odd_width_pad assignments, replace
    conditional terms in addrp_width adjustment by explicit if clauses

 misc/ss.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index fb80d84122fc..56a9ad415ce1 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -107,6 +107,7 @@ int netid_width;
 int state_width;
 int addr_width;
 int serv_width;
+char *odd_width_pad = "";
 
 static const char *TCP_PROTO = "tcp";
 static const char *SCTP_PROTO = "sctp";
@@ -838,7 +839,7 @@ static void sock_state_print(struct sockstat *s)
 			printf("%-*s ", state_width, sstate_name[s->state]);
 	}
 
-	printf("%-6d %-6d ", s->rq, s->wq);
+	printf("%-6d %-6d %s", s->rq, s->wq, odd_width_pad);
 }
 
 static void sock_details_print(struct sockstat *s)
@@ -4364,8 +4365,10 @@ int main(int argc, char *argv[])
 	}
 
 	addrp_width = screen_width;
-	addrp_width -= netid_width+1;
-	addrp_width -= state_width+1;
+	if (netid_width)
+		addrp_width -= netid_width + 1;
+	if (state_width)
+		addrp_width -= state_width + 1;
 	addrp_width -= 14;
 
 	if (addrp_width&1) {
@@ -4373,6 +4376,8 @@ int main(int argc, char *argv[])
 			netid_width++;
 		else if (state_width)
 			state_width++;
+		else
+			odd_width_pad = " ";
 	}
 
 	addrp_width /= 2;
@@ -4390,7 +4395,7 @@ int main(int argc, char *argv[])
 			printf("%-*s ", netid_width, "Netid");
 		if (state_width)
 			printf("%-*s ", state_width, "State");
-		printf("%-6s %-6s ", "Recv-Q", "Send-Q");
+		printf("%-6s %-6s %s", "Recv-Q", "Send-Q", odd_width_pad);
 	}
 
 	/* Make enough space for the local/remote port field */
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-10-31 17:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-31 17:47 [iproute PATCH v2 0/3] ss: netlink_show_one() clean-up, minor output fix Stefano Brivio
2017-10-31 17:47 ` [iproute PATCH v2 1/3] ss: Remove useless width specifier in process context print Stefano Brivio
2017-10-31 17:47 ` [iproute PATCH v2 2/3] ss: Streamline process context printing in netlink_show_one() Stefano Brivio
2017-10-31 17:47 ` [iproute PATCH v2 3/3] ss: Fix width calculations when Netid or State columns are missing Stefano Brivio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).