[PATCH] can: fix proc/can/net/rcvlist_* header alignment on 64-bit system

[PATCH] can: fix proc/can/net/rcvlist_* header alignment on 64-bit system

[Erik Flodin]

Before this fix, the function and userdata columns weren't aligned:
  device   can_id   can_mask  function  userdata   matches  ident
   vcan0  92345678  9fffffff  0000000000000000  0000000000000000         0  raw
   vcan0     123    00000123  0000000000000000  0000000000000000         0  raw

After the fix they are:
  device   can_id   can_mask      function          userdata       matches  ident
   vcan0  92345678  9fffffff  0000000000000000  0000000000000000         0  raw
   vcan0     123    00000123  0000000000000000  0000000000000000         0  raw

Signed-off-by: Erik Flodin <erik@flodin.me>
---
 net/can/proc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/can/proc.c b/net/can/proc.c
index 5ea8695f507e..97901e56c429 100644
--- a/net/can/proc.c
+++ b/net/can/proc.c
@@ -205,8 +205,11 @@ static void can_print_recv_banner(struct seq_file *m)
 	 *                  can1.  00000000  00000000  00000000
 	 *                 .......          0  tp20
 	 */
-	seq_puts(m, "  device   can_id   can_mask  function"
-			"  userdata   matches  ident\n");
+	const char *pad = sizeof(void *) == 8 ? "    " : "";
+
+	seq_printf(m, "  device   can_id   can_mask  %sfunction%s"
+		   "  %suserdata%s   matches  ident\n",
+		   pad, pad, pad, pad);
 }
 
 static int can_stats_proc_show(struct seq_file *m, void *v)
-- 
2.31.0

Re: [PATCH] can: fix proc/can/net/rcvlist_* header alignment on 64-bit system

[Marc Kleine-Budde]

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

Hey Erik,

the subject is not 100% correct, actually it is /proc/net/can/rcvlist_*

On 25.04.2021 10:49:29, Erik Flodin wrote:
> Before this fix, the function and userdata columns weren't aligned:
>   device   can_id   can_mask  function  userdata   matches  ident
>    vcan0  92345678  9fffffff  0000000000000000  0000000000000000         0  raw
>    vcan0     123    00000123  0000000000000000  0000000000000000         0  raw
> 
> After the fix they are:
>   device   can_id   can_mask      function          userdata       matches  ident
>    vcan0  92345678  9fffffff  0000000000000000  0000000000000000         0  raw
>    vcan0     123    00000123  0000000000000000  0000000000000000         0  raw
> 
> Signed-off-by: Erik Flodin <erik@flodin.me>
> ---
>  net/can/proc.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/net/can/proc.c b/net/can/proc.c
> index 5ea8695f507e..97901e56c429 100644
> --- a/net/can/proc.c
> +++ b/net/can/proc.c
> @@ -205,8 +205,11 @@ static void can_print_recv_banner(struct seq_file *m)
>  	 *                  can1.  00000000  00000000  00000000
>  	 *                 .......          0  tp20
>  	 */
> -	seq_puts(m, "  device   can_id   can_mask  function"
> -			"  userdata   matches  ident\n");
> +	const char *pad = sizeof(void *) == 8 ? "    " : "";

nitpick: please move this to the beginning of the function, even before
the comment.

> +
> +	seq_printf(m, "  device   can_id   can_mask  %sfunction%s"
> +		   "  %suserdata%s   matches  ident\n",

nitpick:
For printed strings it's better to have them in a single line, so that
grepping for them is easier.

> +		   pad, pad, pad, pad);
>  }
>  
>  static int can_stats_proc_show(struct seq_file *m, void *v)
> -- 
> 2.31.0
> 
> 

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 --]

[PATCH] can: proc: fix rcvlist_* header alignment on 64-bit system

[Erik Flodin]

Before this fix, the function and userdata columns weren't aligned:
  device   can_id   can_mask  function  userdata   matches  ident
   vcan0  92345678  9fffffff  0000000000000000  0000000000000000         0  raw
   vcan0     123    00000123  0000000000000000  0000000000000000         0  raw

After the fix they are:
  device   can_id   can_mask      function          userdata       matches  ident
   vcan0  92345678  9fffffff  0000000000000000  0000000000000000         0  raw
   vcan0     123    00000123  0000000000000000  0000000000000000         0  raw

Signed-off-by: Erik Flodin <erik@flodin.me>
---
 net/can/proc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/can/proc.c b/net/can/proc.c
index 5ea8695f507e..9c341ccd097c 100644
--- a/net/can/proc.c
+++ b/net/can/proc.c
@@ -201,12 +201,14 @@ static void can_print_rcvlist(struct seq_file *m, struct hlist_head *rx_list,
 
 static void can_print_recv_banner(struct seq_file *m)
 {
+	const char *pad = sizeof(void *) == 8 ? "    " : "";
+
 	/*
 	 *                  can1.  00000000  00000000  00000000
 	 *                 .......          0  tp20
 	 */
-	seq_puts(m, "  device   can_id   can_mask  function"
-			"  userdata   matches  ident\n");
+	seq_printf(m, "  device   can_id   can_mask  %sfunction%s  %suserdata%s   matches  ident\n",
+		   pad, pad, pad, pad);
 }
 
 static int can_stats_proc_show(struct seq_file *m, void *v)
-- 
2.31.0

Re: [PATCH] can: fix proc/can/net/rcvlist_* header alignment on 64-bit system

[Erik Flodin]

Hi,

On Sun, 25 Apr 2021 at 11:07, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> the subject is not 100% correct, actually it is /proc/net/can/rcvlist_*

Good catch. That was sloppy of me...

> nitpick:
> For printed strings it's better to have them in a single line, so that
> grepping for them is easier.

I wasn't sure if I should keep the existing layout or change it since
the include %s probably makes it hard(er) to grep anyway. But I've
changed it in the fixed patch.

Thanks for your comments!

// Erik

Re: [PATCH] can: proc: fix rcvlist_* header alignment on 64-bit system

[Erik Flodin]

Hi,

On Sun, 25 Apr 2021 at 11:53, Erik Flodin <erik@flodin.me> wrote:
> -       seq_puts(m, "  device   can_id   can_mask  function"
> -                       "  userdata   matches  ident\n");
> +       seq_printf(m, "  device   can_id   can_mask  %sfunction%s  %suserdata%s   matches  ident\n",
> +                  pad, pad, pad, pad);
>  }

If a compile-time variant is better I'm happy to change this to e.g.
something like this:

seq_puts(m, "  device   can_id   can_mask  ");
if (IS_ENABLED(CONFIG_64BIT))
        seq_puts(m, "    function          userdata    ");
else
        seq_puts(m, "function  userdata");
seq_puts(m, "   matches  ident\n");

or something like what Vincent suggested:

#ifdef CONFIG_64BIT
#define PAD "    "
#else
#define PAD ""
#endif
...
seq_puts(m, "  device   can_id   can_mask  " PAD "function  " PAD
PAD "userdata   " PAD "matches  ident\n");

None of these versions are really grep friendly though. If that is
needed, a third variant with two full strings can be used instead.
Just let me know which one that's preferred.

// Erik

Re: [PATCH] can: proc: fix rcvlist_* header alignment on 64-bit system

[Vincent MAILHOL]

On Sun. 25 Apr 2021 at 20:40, Erik Flodin <erik@flodin.me> wrote:
>
> None of these versions are really grep friendly though. If that is
> needed, a third variant with two full strings can be used instead.
> Just let me know which one that's preferred.

Out of all the propositions, my favorite is the third variant
with two full strings.  It is optimal in terms of computing
time (not that this is a bottleneck...), it can be grepped and
the source code is easy to understand.

Re: [PATCH] can: proc: fix rcvlist_* header alignment on 64-bit system

[Marc Kleine-Budde]

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

On 25.04.2021 21:05:41, Vincent MAILHOL wrote:
> On Sun. 25 Apr 2021 at 20:40, Erik Flodin <erik@flodin.me> wrote:
> >
> > None of these versions are really grep friendly though. If that is
> > needed, a third variant with two full strings can be used instead.
> > Just let me know which one that's preferred.
> 
> Out of all the propositions, my favorite is the third variant
> with two full strings.  It is optimal in terms of computing
> time (not that this is a bottleneck...), it can be grepped and
> the source code is easy to understand.

+1

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 --]

[PATCH v3] can: proc: fix rcvlist_* header alignment on 64-bit system

[Erik Flodin]

Before this fix, the function and userdata columns weren't aligned:
  device   can_id   can_mask  function  userdata   matches  ident
   vcan0  92345678  9fffffff  0000000000000000  0000000000000000         0  raw
   vcan0     123    00000123  0000000000000000  0000000000000000         0  raw

After the fix they are:
  device   can_id   can_mask      function          userdata       matches  ident
   vcan0  92345678  9fffffff  0000000000000000  0000000000000000         0  raw
   vcan0     123    00000123  0000000000000000  0000000000000000         0  raw

Signed-off-by: Erik Flodin <erik@flodin.me>
---
 net/can/proc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/can/proc.c b/net/can/proc.c
index 5ea8695f507e..35b6c7512785 100644
--- a/net/can/proc.c
+++ b/net/can/proc.c
@@ -205,8 +205,11 @@ static void can_print_recv_banner(struct seq_file *m)
 	 *                  can1.  00000000  00000000  00000000
 	 *                 .......          0  tp20
 	 */
-	seq_puts(m, "  device   can_id   can_mask  function"
-			"  userdata   matches  ident\n");
+#ifdef CONFIG_64BIT
+	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");
+#endif
 }
 
 static int can_stats_proc_show(struct seq_file *m, void *v)
-- 
2.31.0

Re: [PATCH v3] can: proc: fix rcvlist_* header alignment on 64-bit system

[Marc Kleine-Budde]

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

On 25.04.2021 14:22:12, Erik Flodin wrote:
> Before this fix, the function and userdata columns weren't aligned:
>   device   can_id   can_mask  function  userdata   matches  ident
>    vcan0  92345678  9fffffff  0000000000000000  0000000000000000         0  raw
>    vcan0     123    00000123  0000000000000000  0000000000000000         0  raw
> 
> After the fix they are:
>   device   can_id   can_mask      function          userdata       matches  ident
>    vcan0  92345678  9fffffff  0000000000000000  0000000000000000         0  raw
>    vcan0     123    00000123  0000000000000000  0000000000000000         0  raw
> 
> Signed-off-by: Erik Flodin <erik@flodin.me>
> ---
>  net/can/proc.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/net/can/proc.c b/net/can/proc.c
> index 5ea8695f507e..35b6c7512785 100644
> --- a/net/can/proc.c
> +++ b/net/can/proc.c
> @@ -205,8 +205,11 @@ static void can_print_recv_banner(struct seq_file *m)
>  	 *                  can1.  00000000  00000000  00000000
>  	 *                 .......          0  tp20
>  	 */
> -	seq_puts(m, "  device   can_id   can_mask  function"
> -			"  userdata   matches  ident\n");
> +#ifdef CONFIG_64BIT
> +	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");
> +#endif

Please use "if (IS_ENABLED(CONFIG_64BIT))" as in your example in your
previous mail.

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 --]

[PATCH] can: proc: fix rcvlist_* header alignment on 64-bit system

[Erik Flodin]

Before this fix, the function and userdata columns weren't aligned:
  device   can_id   can_mask  function  userdata   matches  ident
   vcan0  92345678  9fffffff  0000000000000000  0000000000000000         0  raw
   vcan0     123    00000123  0000000000000000  0000000000000000         0  raw

After the fix they are:
  device   can_id   can_mask      function          userdata       matches  ident
   vcan0  92345678  9fffffff  0000000000000000  0000000000000000         0  raw
   vcan0     123    00000123  0000000000000000  0000000000000000         0  raw

Signed-off-by: Erik Flodin <erik@flodin.me>
---
 net/can/proc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/can/proc.c b/net/can/proc.c
index 5ea8695f507e..ba00619cc3c0 100644
--- a/net/can/proc.c
+++ b/net/can/proc.c
@@ -205,8 +205,10 @@ static void can_print_recv_banner(struct seq_file *m)
 	 *                  can1.  00000000  00000000  00000000
 	 *                 .......          0  tp20
 	 */
-	seq_puts(m, "  device   can_id   can_mask  function"
-			"  userdata   matches  ident\n");
+	if (IS_ENABLED(CONFIG_64BIT))
+		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");
 }
 
 static int can_stats_proc_show(struct seq_file *m, void *v)
-- 
2.31.0

Re: [PATCH v3] can: proc: fix rcvlist_* header alignment on 64-bit system

[Erik Flodin]

Hi,

On Sun, 25 Apr 2021 at 14:27, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
>
> On 25.04.2021 14:22:12, Erik Flodin wrote:
> > Before this fix, the function and userdata columns weren't aligned:
> >   device   can_id   can_mask  function  userdata   matches  ident
> >    vcan0  92345678  9fffffff  0000000000000000  0000000000000000         0  raw
> >    vcan0     123    00000123  0000000000000000  0000000000000000         0  raw
> >
> > After the fix they are:
> >   device   can_id   can_mask      function          userdata       matches  ident
> >    vcan0  92345678  9fffffff  0000000000000000  0000000000000000         0  raw
> >    vcan0     123    00000123  0000000000000000  0000000000000000         0  raw
> >
> > Signed-off-by: Erik Flodin <erik@flodin.me>
> > ---
> >  net/can/proc.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/can/proc.c b/net/can/proc.c
> > index 5ea8695f507e..35b6c7512785 100644
> > --- a/net/can/proc.c
> > +++ b/net/can/proc.c
> > @@ -205,8 +205,11 @@ static void can_print_recv_banner(struct seq_file *m)
> >        *                  can1.  00000000  00000000  00000000
> >        *                 .......          0  tp20
> >        */
> > -     seq_puts(m, "  device   can_id   can_mask  function"
> > -                     "  userdata   matches  ident\n");
> > +#ifdef CONFIG_64BIT
> > +     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");
> > +#endif
>
> Please use "if (IS_ENABLED(CONFIG_64BIT))" as in your example in your
> previous mail.

Ok. I've sent a new patch, but of course I forgot to add -v4. Sorry about that.

// Erik