Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH iproute2] ss: fix end-of-line printing in misc/ss.c
@ 2019-11-27  5:21 Brian Vazquez
  2019-12-03 13:00 ` Michal Kubecek
  2019-12-04 19:11 ` Stephen Hemminger
  0 siblings, 2 replies; 4+ messages in thread
From: Brian Vazquez @ 2019-11-27  5:21 UTC (permalink / raw)
  To: Brian Vazquez, David Ahern, Stephen Hemminger
  Cc: Mahesh Bandewar, Maciej Zenczykowski, netdev, Brian Vazquez,
	Hritik Vijay

Before commit 5883c6eba517, function field_is_last() was incorrectly
reporting which column was the last because it was missing COL_PROC
and by purely coincidence it was correctly printing the end-of-line and
moving to the first column since the very last field was empty, and
end-of-line was added for the last non-empty token since it was seen as
the last field.

This commits correcrly prints the end-of-line for the last entrien in
the ss command.

Tested:
diff <(./ss.old -nltp) <(misc/ss -nltp)
38c38
< LISTEN    0   128     [::1]:35417   [::]:*   users:(("foo",pid=65254,fd=116))
\ No newline at end of file
---
> LISTEN    0   128     [::1]:35417   [::]:*   users:(("foo",pid=65254,fd=116))

Cc: Hritik Vijay <hritikxx8@gmail.com>
Fixes: 5883c6eba517 ("ss: show header for --processes/-p")
Signed-off-by: Brian Vazquez <brianvv@google.com>
---
 misc/ss.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/misc/ss.c b/misc/ss.c
index c58e5c4d..95f1d37a 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1290,6 +1290,11 @@ static void render(void)
 
 		token = buf_token_next(token);
 	}
+	/* Deal with final end-of-line when the last non-empty field printed
+	 * is not the last field.
+	 */
+	if (line_started)
+		printf("\n");
 
 	buf_free_all();
 	current_field = columns;
-- 
2.24.0.432.g9d3f5f5b63-goog


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

* Re: [PATCH iproute2] ss: fix end-of-line printing in misc/ss.c
  2019-11-27  5:21 [PATCH iproute2] ss: fix end-of-line printing in misc/ss.c Brian Vazquez
@ 2019-12-03 13:00 ` Michal Kubecek
  2019-12-04 19:11 ` Stephen Hemminger
  1 sibling, 0 replies; 4+ messages in thread
From: Michal Kubecek @ 2019-12-03 13:00 UTC (permalink / raw)
  To: netdev
  Cc: Brian Vazquez, Brian Vazquez, David Ahern, Stephen Hemminger,
	Mahesh Bandewar, Maciej Zenczykowski, Hritik Vijay

On Tue, Nov 26, 2019 at 09:21:18PM -0800, Brian Vazquez wrote:
> Before commit 5883c6eba517, function field_is_last() was incorrectly
> reporting which column was the last because it was missing COL_PROC
> and by purely coincidence it was correctly printing the end-of-line and
> moving to the first column since the very last field was empty, and
> end-of-line was added for the last non-empty token since it was seen as
> the last field.
> 
> This commits correcrly prints the end-of-line for the last entrien in
> the ss command.
> 
> Tested:
> diff <(./ss.old -nltp) <(misc/ss -nltp)
> 38c38
> < LISTEN    0   128     [::1]:35417   [::]:*   users:(("foo",pid=65254,fd=116))
> \ No newline at end of file
> ---
> > LISTEN    0   128     [::1]:35417   [::]:*   users:(("foo",pid=65254,fd=116))
> 
> Cc: Hritik Vijay <hritikxx8@gmail.com>
> Fixes: 5883c6eba517 ("ss: show header for --processes/-p")
> Signed-off-by: Brian Vazquez <brianvv@google.com>

Tested-by: Michal Kubecek <mkubecek@suse.cz>

> ---
>  misc/ss.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/misc/ss.c b/misc/ss.c
> index c58e5c4d..95f1d37a 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -1290,6 +1290,11 @@ static void render(void)
>  
>  		token = buf_token_next(token);
>  	}
> +	/* Deal with final end-of-line when the last non-empty field printed
> +	 * is not the last field.
> +	 */
> +	if (line_started)
> +		printf("\n");
>  
>  	buf_free_all();
>  	current_field = columns;
> -- 
> 2.24.0.432.g9d3f5f5b63-goog
> 

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

* Re: [PATCH iproute2] ss: fix end-of-line printing in misc/ss.c
  2019-11-27  5:21 [PATCH iproute2] ss: fix end-of-line printing in misc/ss.c Brian Vazquez
  2019-12-03 13:00 ` Michal Kubecek
@ 2019-12-04 19:11 ` Stephen Hemminger
  2019-12-04 19:39   ` Brian Vazquez
  1 sibling, 1 reply; 4+ messages in thread
From: Stephen Hemminger @ 2019-12-04 19:11 UTC (permalink / raw)
  To: Brian Vazquez
  Cc: Brian Vazquez, David Ahern, Mahesh Bandewar, Maciej Zenczykowski,
	netdev, Hritik Vijay

On Tue, 26 Nov 2019 21:21:18 -0800
Brian Vazquez <brianvv@google.com> wrote:

> Before commit 5883c6eba517, function field_is_last() was incorrectly
> reporting which column was the last because it was missing COL_PROC
> and by purely coincidence it was correctly printing the end-of-line and
> moving to the first column since the very last field was empty, and
> end-of-line was added for the last non-empty token since it was seen as
> the last field.
> 
> This commits correcrly prints the end-of-line for the last entrien in
> the ss command.
> 
> Tested:
> diff <(./ss.old -nltp) <(misc/ss -nltp)
> 38c38
> < LISTEN    0   128     [::1]:35417   [::]:*   users:(("foo",pid=65254,fd=116))
> \ No newline at end of file
> ---
> > LISTEN    0   128     [::1]:35417   [::]:*   users:(("foo",pid=65254,fd=116))  
> 
> Cc: Hritik Vijay <hritikxx8@gmail.com>
> Fixes: 5883c6eba517 ("ss: show header for --processes/-p")
> Signed-off-by: Brian Vazquez <brianvv@google.com>

This commit message is really hard to understand and causes warnings
in checkpatch. Also, blaming old code for doing the right thing
is not necessary. The changelog doesn't need to explain why.
The offending commit is already referenced by the fixes line.

Instead, I propose:


The previous change to ss to show header broke the printing of end-of-line
for the last entry.

Fixes: 5883c6eba517 ("ss: show header for --processes/-p")
Signed-off-by: Brian Vazquez <brianvv@google.com>

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

* Re: [PATCH iproute2] ss: fix end-of-line printing in misc/ss.c
  2019-12-04 19:11 ` Stephen Hemminger
@ 2019-12-04 19:39   ` Brian Vazquez
  0 siblings, 0 replies; 4+ messages in thread
From: Brian Vazquez @ 2019-12-04 19:39 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Brian Vazquez, David Ahern, Mahesh Bandewar, Maciej Zenczykowski,
	Linux NetDev, Hritik Vijay

Thanks for reviewing it!

On Wed, Dec 4, 2019 at 11:11 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 26 Nov 2019 21:21:18 -0800
> Brian Vazquez <brianvv@google.com> wrote:
>
> > Before commit 5883c6eba517, function field_is_last() was incorrectly
> > reporting which column was the last because it was missing COL_PROC
> > and by purely coincidence it was correctly printing the end-of-line and
> > moving to the first column since the very last field was empty, and
> > end-of-line was added for the last non-empty token since it was seen as
> > the last field.
> >
> > This commits correcrly prints the end-of-line for the last entrien in
> > the ss command.
> >
> > Tested:
> > diff <(./ss.old -nltp) <(misc/ss -nltp)
> > 38c38
> > < LISTEN    0   128     [::1]:35417   [::]:*   users:(("foo",pid=65254,fd=116))
> > \ No newline at end of file
> > ---
> > > LISTEN    0   128     [::1]:35417   [::]:*   users:(("foo",pid=65254,fd=116))
> >
> > Cc: Hritik Vijay <hritikxx8@gmail.com>
> > Fixes: 5883c6eba517 ("ss: show header for --processes/-p")
> > Signed-off-by: Brian Vazquez <brianvv@google.com>
>
> This commit message is really hard to understand and causes warnings
> in checkpatch. Also, blaming old code for doing the right thing
> is not necessary. The changelog doesn't need to explain why.
> The offending commit is already referenced by the fixes line.
>
> Instead, I propose:
>
>
> The previous change to ss to show header broke the printing of end-of-line
> for the last entry.

This makes sense, I'll fix it in next version. Thanks!

>
> Fixes: 5883c6eba517 ("ss: show header for --processes/-p")
> Signed-off-by: Brian Vazquez <brianvv@google.com>

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-27  5:21 [PATCH iproute2] ss: fix end-of-line printing in misc/ss.c Brian Vazquez
2019-12-03 13:00 ` Michal Kubecek
2019-12-04 19:11 ` Stephen Hemminger
2019-12-04 19:39   ` Brian Vazquez

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git