All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] can: proc: properly format table
@ 2023-02-06 19:43 Marc Kleine-Budde
  2023-02-06 19:45 ` Marc Kleine-Budde
  0 siblings, 1 reply; 4+ messages in thread
From: Marc Kleine-Budde @ 2023-02-06 19:43 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Marc Kleine-Budde, Oliver Hartkopp

The table "/proc/net/can/rcvlist_all" is garbled if the interface
names are longer than 5 characters.

Consider IFNAMSIZ when formatting the table so that it looks like
this:

| receive list 'rx_all':
|   device              can_id   can_mask  function  userdata   matches  ident
|    any                   000   00000000  8e807747  9bc49fd8         0  raw
|   device              can_id   can_mask  function  userdata   matches  ident
|    mcp251xfd0            000   00000000  8e807747  ec6d80a2
|   0  raw

Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/proc.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/can/proc.c b/net/can/proc.c
index bbce97825f13..62eeae453544 100644
--- a/net/can/proc.c
+++ b/net/can/proc.c
@@ -186,12 +186,12 @@ static void can_print_rcvlist(struct seq_file *m, struct hlist_head *rx_list,
 	struct receiver *r;
 
 	hlist_for_each_entry_rcu(r, rx_list, list) {
-		char *fmt = (r->can_id & CAN_EFF_FLAG)?
-			"   %-5s  %08x  %08x  %pK  %pK  %8ld  %s\n" :
-			"   %-5s     %03x    %08x  %pK  %pK  %8ld  %s\n";
+		const char *fmt = r->can_id & CAN_EFF_FLAG ?
+			"   %-*s   %08x %08x  %pK  %pK  %8ld  %s\n" :
+			"   %-*s      %03x   %08x  %pK  %pK  %8ld  %s\n";
 
-		seq_printf(m, fmt, DNAME(dev), r->can_id, r->mask,
-				r->func, r->data, r->matches, r->ident);
+		seq_printf(m, fmt, IFNAMSIZ, DNAME(dev), r->can_id, r->mask,
+			   r->func, r->data, r->matches, r->ident);
 	}
 }
 
@@ -202,9 +202,9 @@ static void can_print_recv_banner(struct seq_file *m)
 	 *                 .......          0  tp20
 	 */
 	if (IS_ENABLED(CONFIG_64BIT))
-		seq_puts(m, "  device   can_id   can_mask      function          userdata       matches  ident\n");
+		seq_puts(m, "  device              can_id   can_mask      function          userdata       matches  ident\n");
 	else
-		seq_puts(m, "  device   can_id   can_mask  function  userdata   matches  ident\n");
+		seq_puts(m, "  device              can_id   can_mask  function  userdata   matches  ident\n");
 }
 
 static int can_stats_proc_show(struct seq_file *m, void *v)
-- 
2.39.1



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

* Re: [PATCH] can: proc: properly format table
  2023-02-06 19:43 [PATCH] can: proc: properly format table Marc Kleine-Budde
@ 2023-02-06 19:45 ` Marc Kleine-Budde
  2023-02-07  9:17   ` Oliver Hartkopp
  0 siblings, 1 reply; 4+ messages in thread
From: Marc Kleine-Budde @ 2023-02-06 19:45 UTC (permalink / raw)
  To: linux-can; +Cc: kernel, Oliver Hartkopp

[-- Attachment #1: Type: text/plain, Size: 941 bytes --]

On 06.02.2023 20:43:05, Marc Kleine-Budde wrote:
> The table "/proc/net/can/rcvlist_all" is garbled if the interface
> names are longer than 5 characters.
> 
> Consider IFNAMSIZ when formatting the table so that it looks like
> this:
> 
> | receive list 'rx_all':
> |   device              can_id   can_mask  function  userdata   matches  ident
> |    any                   000   00000000  8e807747  9bc49fd8         0  raw
> |   device              can_id   can_mask  function  userdata   matches  ident
> |    mcp251xfd0            000   00000000  8e807747  ec6d80a2
> |   0  raw
      ^^^^^^
Doh! That's my fault, the proc output is OK. fixed.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH] can: proc: properly format table
  2023-02-06 19:45 ` Marc Kleine-Budde
@ 2023-02-07  9:17   ` Oliver Hartkopp
  2023-02-07 11:40     ` Oliver Hartkopp
  0 siblings, 1 reply; 4+ messages in thread
From: Oliver Hartkopp @ 2023-02-07  9:17 UTC (permalink / raw)
  To: Marc Kleine-Budde, linux-can; +Cc: kernel

Hi Marc,

not sure on which architecture your proc output was created.

On 2/6/23 20:45, Marc Kleine-Budde wrote:
> On 06.02.2023 20:43:05, Marc Kleine-Budde wrote:
>> The table "/proc/net/can/rcvlist_all" is garbled if the interface
>> names are longer than 5 characters.
>>
>> Consider IFNAMSIZ when formatting the table so that it looks like
>> this:
>>
>> | receive list 'rx_all':
>> |   device              can_id   can_mask  function  userdata   matches  ident
>> |    any                   000   00000000  8e807747  9bc49fd8         0  raw
>> |   device              can_id   can_mask  function  userdata   matches  ident
>> |    mcp251xfd0            000   00000000  8e807747  ec6d80a2
>> |   0  raw
>        ^^^^^^
> Doh! That's my fault, the proc output is OK. fixed.
> 

On my x86-64 with IS_ENABLED(CONFIG_64BIT) it looks even worse:

cat /proc/net/can/rcvlist_all

receive list 'rx_all':
   (any: no entry)
   device   can_id   can_mask      function          userdata 
matches  ident
    vcan0     000    00000000  00000000cb627637  00000000afdf543a 
  0  raw
   device   can_id   can_mask      function          userdata 
matches  ident
    vcan1     000    00000000  00000000cb627637  0000000020f218f6 
  0  raw

I wonder if we should clean up this proc stuff in general.

As you can see the "function" value is identical and points to "raw_rcv" 
but the pointer is (of course) a pseudonym to not leak any kernel 
pointer to the user space. IMO we could drop this column as the "ident" 
column identifies the can_rx_register() user anyway.

The 'userdata' column is still helpful to identify different socket 
instances while debugging. But maybe we can "fold" the address in a way 
that it is always 32 bit?!?

With such kind of clean up the output of a proc line would fit into 80 
chars again - even with your suggestion for IFNAMSZ.

What do you think about such clean up?

Best regards,
Oliver

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

* Re: [PATCH] can: proc: properly format table
  2023-02-07  9:17   ` Oliver Hartkopp
@ 2023-02-07 11:40     ` Oliver Hartkopp
  0 siblings, 0 replies; 4+ messages in thread
From: Oliver Hartkopp @ 2023-02-07 11:40 UTC (permalink / raw)
  To: Marc Kleine-Budde, linux-can; +Cc: kernel

What about this kind of layout?

           device    can_id  can_mask  userdata  matches  ident
1234567812345678       123  12345678  12345678  1234567  isotpe

           device    can_id  can_mask  userdata  matches  ident
             can0  12345678  12345678  12345678  1234567  isotpe

           device    can_id  can_mask      userdata      matches  ident
1234567812345678       123  12345678  0000000012345678  1234567  isotpe

           device    can_id  can_mask      userdata      matches  ident
       mcp251xfd0  12345678  12345678  0000000012345678  1234567  isotpe

12345678901234567890123456789012345678901234567890123456789012345678901234567890

Only the "matches" could lead to a length beyond 7 characters then and 
would lead to a garbled output at the end of that line.

Best regards,
Oliver

On 2/7/23 10:17, Oliver Hartkopp wrote:
> Hi Marc,
> 
> not sure on which architecture your proc output was created.
> 
> On 2/6/23 20:45, Marc Kleine-Budde wrote:
>> On 06.02.2023 20:43:05, Marc Kleine-Budde wrote:
>>> The table "/proc/net/can/rcvlist_all" is garbled if the interface
>>> names are longer than 5 characters.
>>>
>>> Consider IFNAMSIZ when formatting the table so that it looks like
>>> this:
>>>
>>> | receive list 'rx_all':
>>> |   device              can_id   can_mask  function  userdata   
>>> matches  ident
>>> |    any                   000   00000000  8e807747  9bc49fd8         
>>> 0  raw
>>> |   device              can_id   can_mask  function  userdata   
>>> matches  ident
>>> |    mcp251xfd0            000   00000000  8e807747  ec6d80a2
>>> |   0  raw
>>        ^^^^^^
>> Doh! That's my fault, the proc output is OK. fixed.
>>
> 
> On my x86-64 with IS_ENABLED(CONFIG_64BIT) it looks even worse:
> 
> cat /proc/net/can/rcvlist_all
> 
> receive list 'rx_all':
>    (any: no entry)
>    device   can_id   can_mask      function          userdata matches  
> ident
>     vcan0     000    00000000  00000000cb627637  00000000afdf543a  0  raw
>    device   can_id   can_mask      function          userdata matches  
> ident
>     vcan1     000    00000000  00000000cb627637  0000000020f218f6  0  raw
> 
> I wonder if we should clean up this proc stuff in general.
> 
> As you can see the "function" value is identical and points to "raw_rcv" 
> but the pointer is (of course) a pseudonym to not leak any kernel 
> pointer to the user space. IMO we could drop this column as the "ident" 
> column identifies the can_rx_register() user anyway.
> 
> The 'userdata' column is still helpful to identify different socket 
> instances while debugging. But maybe we can "fold" the address in a way 
> that it is always 32 bit?!?
> 
> With such kind of clean up the output of a proc line would fit into 80 
> chars again - even with your suggestion for IFNAMSZ.
> 
> What do you think about such clean up?
> 
> Best regards,
> Oliver

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

end of thread, other threads:[~2023-02-07 11:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-06 19:43 [PATCH] can: proc: properly format table Marc Kleine-Budde
2023-02-06 19:45 ` Marc Kleine-Budde
2023-02-07  9:17   ` Oliver Hartkopp
2023-02-07 11:40     ` Oliver Hartkopp

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.