[PATCH can-utils-dev 0/5] can-utils-dev: a set of cleanup patches

[PATCH can-utils-dev 0/5] can-utils-dev: a set of cleanup patches

[Vincent Mailhol]

This series contain five cleanup patches. There is no real relation
between each patch so I will end the cover letter here and let you
refer to the short description.

Vincent Mailhol (5):
  slcanpty: remove redundant asc2nibble()
  lib: add pr_debug() macro
  candump: add global variable progname
  candump: use linux/net_tstamp.h instead of redefining values ourselves
  lib: snprintf_can_error_frame: print counter errors if CAN_ERR_CNT is
    set

 Makefile                           |  2 ++
 canbusload.c                       |  9 ++---
 candump.c                          | 58 +++++++++++++-----------------
 canlogserver.c                     | 12 +++----
 lib.c                              |  4 +++
 lib.h                              |  6 ++++
 mcp251xfd/mcp251xfd-dev-coredump.c | 10 +-----
 slcanpty.c                         | 27 +++-----------
 8 files changed, 50 insertions(+), 78 deletions(-)

-- 
2.37.4

[PATCH can-utils-dev 1/5] slcanpty: remove redundant asc2nibble()

[Vincent Mailhol]

asc2nibble() is already defined in lib.h. Include lib.h in slcnpty.c
so that asc2nibble() does not need to be redefined a second time and
adjust the Makefile accordingly.

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 Makefile   |  2 ++
 slcanpty.c | 17 ++---------------
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index fb52643..cb67b66 100644
--- a/Makefile
+++ b/Makefile
@@ -119,6 +119,7 @@ canplayer.o:	lib.h
 cansend.o:	lib.h
 log2asc.o:	lib.h
 log2long.o:	lib.h
+slcanpty.o:	lib.h
 j1939acd.o:	libj1939.h
 j1939cat.o:	libj1939.h
 j1939spy.o:	libj1939.h
@@ -135,6 +136,7 @@ cansend:	cansend.o	lib.o
 cansequence:	cansequence.o	lib.o
 log2asc:	log2asc.o	lib.o
 log2long:	log2long.o	lib.o
+slcanpty:	slcanpty.o	lib.o
 j1939acd:	j1939acd.o	libj1939.o
 j1939cat:	j1939cat.o	libj1939.o
 j1939spy:	j1939spy.o	libj1939.o
diff --git a/slcanpty.c b/slcanpty.c
index e6f1efe..4ac9e8a 100644
--- a/slcanpty.c
+++ b/slcanpty.c
@@ -43,27 +43,14 @@
 #include <linux/can/raw.h>
 #include <linux/sockios.h>
 
+#include "lib.h"
+
 /* maximum rx buffer len: extended CAN frame with timestamp */
 #define SLC_MTU (sizeof("T1111222281122334455667788EA5F\r")+1)
 #define DEVICE_NAME_PTMX "/dev/ptmx"
 
 #define DEBUG
 
-static int asc2nibble(char c)
-{
-
-	if ((c >= '0') && (c <= '9'))
-		return c - '0';
-
-	if ((c >= 'A') && (c <= 'F'))
-		return c - 'A' + 10;
-
-	if ((c >= 'a') && (c <= 'f'))
-		return c - 'a' + 10;
-
-	return 16; /* error */
-}
-
 /* read data from pty, send CAN frames to CAN socket and answer commands */
 int pty2can(int pty, int socket, struct can_filter *fi,
 	    int *is_open, int *tstamp)
-- 
2.37.4

[PATCH can-utils-dev 2/5] lib: add pr_debug() macro

[Vincent Mailhol]

Add the pr_debug() macro so that replace:

  #ifdef DEBUG
  	printf("foo");
  #endif

by:

  	pr_debug("foo");

Apply the pr_debug() macro wherever relevant.

Currently, there is no consensus whether debug messages should be
printed on stdout or stderr. Most of the modules: canbusload.c,
candump.c and canlogserver.c use stdout but
mcp251xfd/mcp251xfd-dev-coredump.c uses stderr. Harmonize the behavior
by following the major trend and make
mcp251xfd/mcp251xfd-dev-coredump.c also output to stdout.

CC: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
@Marc, was there any reasons to print debug information to stderr and
not stdout in mcp251xfd-dev-coredump.c?
---
 canbusload.c                       |  9 +++------
 candump.c                          | 16 ++++------------
 canlogserver.c                     | 12 ++++--------
 lib.h                              |  6 ++++++
 mcp251xfd/mcp251xfd-dev-coredump.c | 10 +---------
 slcanpty.c                         | 10 +++-------
 6 files changed, 21 insertions(+), 42 deletions(-)

diff --git a/canbusload.c b/canbusload.c
index e4dfc02..47b62fd 100644
--- a/canbusload.c
+++ b/canbusload.c
@@ -61,6 +61,7 @@
 #include <linux/can.h>
 #include <linux/can/raw.h>
 
+#include "lib.h"
 #include "terminal.h"
 #include "canframelen.h"
 
@@ -310,9 +311,7 @@ int main(int argc, char **argv)
 			return 1;
 		}
 
-#ifdef DEBUG
-		printf("open %d '%s'.\n", i, ptr);
-#endif
+		pr_debug("open %d '%s'.\n", i, ptr);
 
 		s[i] = socket(PF_CAN, SOCK_RAW, CAN_RAW);
 		if (s[i] < 0) {
@@ -358,10 +357,8 @@ int main(int argc, char **argv)
 		if (nbytes > max_bitrate_len)
 			max_bitrate_len = nbytes; /* for nice printing */
 
+		pr_debug("using interface name '%s'.\n", ifr.ifr_name);
 
-#ifdef DEBUG
-		printf("using interface name '%s'.\n", ifr.ifr_name);
-#endif
 		/* try to switch the socket into CAN FD mode */
 		const int canfd_on = 1;
 		setsockopt(s[i], SOL_CAN_RAW, CAN_RAW_FD_FRAMES, &canfd_on, sizeof(canfd_on));
diff --git a/candump.c b/candump.c
index 6fe30bb..4a239e7 100644
--- a/candump.c
+++ b/candump.c
@@ -216,9 +216,7 @@ static int idx2dindex(int ifidx, int socket)
 
 	strcpy(devname[i], ifr.ifr_name);
 
-#ifdef DEBUG
-	printf("new index %d (%s)\n", i, devname[i]);
-#endif
+	pr_debug("new index %d (%s)\n", i, devname[i]);
 
 	return i;
 }
@@ -474,9 +472,7 @@ int main(int argc, char **argv)
 		ptr = argv[optind+i];
 		nptr = strchr(ptr, ',');
 
-#ifdef DEBUG
-		printf("open %d '%s'.\n", i, ptr);
-#endif
+		pr_debug("open %d '%s'.\n", i, ptr);
 
 		obj->s = socket(PF_CAN, SOCK_RAW, CAN_RAW);
 		if (obj->s < 0) {
@@ -510,9 +506,7 @@ int main(int argc, char **argv)
 		memset(&ifr.ifr_name, 0, sizeof(ifr.ifr_name));
 		strncpy(ifr.ifr_name, ptr, nbytes);
 
-#ifdef DEBUG
-		printf("using interface name '%s'.\n", ifr.ifr_name);
-#endif
+		pr_debug("using interface name '%s'.\n", ifr.ifr_name);
 
 		if (strcmp(ANYDEV, ifr.ifr_name) != 0) {
 			if (ioctl(obj->s, SIOCGIFINDEX, &ifr) < 0) {
@@ -602,9 +596,7 @@ int main(int argc, char **argv)
 			/* try SO_RCVBUFFORCE first, if we run with CAP_NET_ADMIN */
 			if (setsockopt(obj->s, SOL_SOCKET, SO_RCVBUFFORCE,
 				       &rcvbuf_size, sizeof(rcvbuf_size)) < 0) {
-#ifdef DEBUG
-				printf("SO_RCVBUFFORCE failed so try SO_RCVBUF ...\n");
-#endif
+				pr_debug("SO_RCVBUFFORCE failed so try SO_RCVBUF ...\n");
 				if (setsockopt(obj->s, SOL_SOCKET, SO_RCVBUF,
 					       &rcvbuf_size, sizeof(rcvbuf_size)) < 0) {
 					perror("setsockopt SO_RCVBUF");
diff --git a/canlogserver.c b/canlogserver.c
index 6425ca4..51d548f 100644
--- a/canlogserver.c
+++ b/canlogserver.c
@@ -145,9 +145,7 @@ int idx2dindex(int ifidx, int socket)
 
 	strcpy(devname[i], ifr.ifr_name);
 
-#ifdef DEBUG
-	printf("new index %d (%s)\n", i, devname[i]);
-#endif
+	pr_debug("new index %d (%s)\n", i, devname[i]);
 
 	return i;
 }
@@ -310,11 +308,9 @@ int main(int argc, char **argv)
 
 	for (i=0; i<currmax; i++) {
 
-#ifdef DEBUG
-		printf("open %d '%s' m%08X v%08X i%d e%d.\n",
-		       i, argv[optind+i], mask[i], value[i],
-		       inv_filter[i], err_mask[i]);
-#endif
+		pr_debug("open %d '%s' m%08X v%08X i%d e%d.\n",
+		      i, argv[optind+i], mask[i], value[i],
+		      inv_filter[i], err_mask[i]);
 
 		if ((s[i] = socket(PF_CAN, SOCK_RAW, CAN_RAW)) < 0) {
 			perror("socket");
diff --git a/lib.h b/lib.h
index a4d3ce5..1cb1dd4 100644
--- a/lib.h
+++ b/lib.h
@@ -47,6 +47,12 @@
 
 #include <stdio.h>
 
+#ifdef DEBUG
+#define pr_debug(fmt, args...) printf(fmt, ##args)
+#else
+#define pr_debug(...)
+#endif
+
 /* buffer sizes for CAN frame string representations */
 
 #define CL_ID (sizeof("12345678##1"))
diff --git a/mcp251xfd/mcp251xfd-dev-coredump.c b/mcp251xfd/mcp251xfd-dev-coredump.c
index 5874d24..422900f 100644
--- a/mcp251xfd/mcp251xfd-dev-coredump.c
+++ b/mcp251xfd/mcp251xfd-dev-coredump.c
@@ -17,18 +17,10 @@
 
 #include <linux/kernel.h>
 
+#include "../lib.h"
 #include "mcp251xfd.h"
 #include "mcp251xfd-dump-userspace.h"
 
-#define pr_err(fmt, args...)    fprintf(stderr, fmt, ##args)
-#define pr_no(fmt, args...)     while (0) { fprintf(stdout, fmt, ##args); }
-
-#ifdef DEBUG
-#define pr_debug(fmt, args...) pr_err(fmt, ##args)
-#else
-#define pr_debug(fmt, args...) pr_no(fmt, ##args)
-#endif
-
 
 struct mcp251xfd_dump_iter {
 	const void *start;
diff --git a/slcanpty.c b/slcanpty.c
index 4ac9e8a..3eba07e 100644
--- a/slcanpty.c
+++ b/slcanpty.c
@@ -49,8 +49,6 @@
 #define SLC_MTU (sizeof("T1111222281122334455667788EA5F\r")+1)
 #define DEVICE_NAME_PTMX "/dev/ptmx"
 
-#define DEBUG
-
 /* read data from pty, send CAN frames to CAN socket and answer commands */
 int pty2can(int pty, int socket, struct can_filter *fi,
 	    int *is_open, int *tstamp)
@@ -106,14 +104,12 @@ rx_restart:
 	cmd = buf[0];
 	buf[nbytes] = 0;
 
-#ifdef DEBUG
 	for (tmp = 0; tmp < nbytes; tmp++)
 		if (buf[tmp] == '\r')
-			putchar('@');
+			pr_debug("@");
 		else
-			putchar(buf[tmp]);
-	printf("\n");
-#endif
+			pr_debug(%c, buf[tmp]);
+	pr_debug("\n");
 
 	/* check for filter configuration commands */
 	if (cmd == 'm' || cmd == 'M') {
-- 
2.37.4

Re: [PATCH can-utils-dev 2/5] lib: add pr_debug() macro

[Marc Kleine-Budde]

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

On 13.11.2022 17:53:18, Vincent Mailhol wrote:
> Add the pr_debug() macro so that replace:
> 
>   #ifdef DEBUG
>   	printf("foo");
>   #endif
> 
> by:
> 
>   	pr_debug("foo");
> 
> Apply the pr_debug() macro wherever relevant.
> 
> Currently, there is no consensus whether debug messages should be
> printed on stdout or stderr. Most of the modules: canbusload.c,
> candump.c and canlogserver.c use stdout but
> mcp251xfd/mcp251xfd-dev-coredump.c uses stderr. Harmonize the behavior
> by following the major trend and make
> mcp251xfd/mcp251xfd-dev-coredump.c also output to stdout.
> 
> CC: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> ---
> @Marc, was there any reasons to print debug information to stderr and
> not stdout in mcp251xfd-dev-coredump.c?

No real reason, just gut feeling. There are at least 2 differences: IIRC
stdout is line buffered, it will not write to console until a newline.
stderr will print even if there is no newline. The other one is: If
use/parse the stdout if the program you're debugging (i.e. in a pipe)
the debug output on stdout will interfere with the regular output.

[...]

> diff --git a/lib.h b/lib.h
> index a4d3ce5..1cb1dd4 100644
> --- a/lib.h
> +++ b/lib.h
> @@ -47,6 +47,12 @@
>  
>  #include <stdio.h>
>  
> +#ifdef DEBUG
> +#define pr_debug(fmt, args...) printf(fmt, ##args)
> +#else
> +#define pr_debug(...)
> +#endif

With this change if DEBUG is not defined, the print format strings are
not checked. This is why I have the pr_no macro. Side node: For functions
you can use __attribute__ ((format (printf, 2, 3))).

> +
>  /* buffer sizes for CAN frame string representations */
>  
>  #define CL_ID (sizeof("12345678##1"))
> diff --git a/mcp251xfd/mcp251xfd-dev-coredump.c b/mcp251xfd/mcp251xfd-dev-coredump.c
> index 5874d24..422900f 100644
> --- a/mcp251xfd/mcp251xfd-dev-coredump.c
> +++ b/mcp251xfd/mcp251xfd-dev-coredump.c
> @@ -17,18 +17,10 @@
>  
>  #include <linux/kernel.h>
>  
> +#include "../lib.h"
>  #include "mcp251xfd.h"
>  #include "mcp251xfd-dump-userspace.h"
>  
> -#define pr_err(fmt, args...)    fprintf(stderr, fmt, ##args)
> -#define pr_no(fmt, args...)     while (0) { fprintf(stdout, fmt, ##args); }
> -
> -#ifdef DEBUG
> -#define pr_debug(fmt, args...) pr_err(fmt, ##args)
> -#else
> -#define pr_debug(fmt, args...) pr_no(fmt, ##args)
> -#endif
> -
>  
>  struct mcp251xfd_dump_iter {
>  	const void *start;
> diff --git a/slcanpty.c b/slcanpty.c
> index 4ac9e8a..3eba07e 100644
> --- a/slcanpty.c
> +++ b/slcanpty.c
> @@ -49,8 +49,6 @@
>  #define SLC_MTU (sizeof("T1111222281122334455667788EA5F\r")+1)
>  #define DEVICE_NAME_PTMX "/dev/ptmx"
>  
> -#define DEBUG

For completeness mention that you switch off debugging in slcanpty.c.

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

Re: [PATCH can-utils-dev 2/5] lib: add pr_debug() macro

[Vincent MAILHOL]

On Sun. 13 Nov. 2022 at 21:18, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 13.11.2022 17:53:18, Vincent Mailhol wrote:
> > Add the pr_debug() macro so that replace:
> >
> >   #ifdef DEBUG
> >       printf("foo");
> >   #endif
> >
> > by:
> >
> >       pr_debug("foo");
> >
> > Apply the pr_debug() macro wherever relevant.
> >
> > Currently, there is no consensus whether debug messages should be
> > printed on stdout or stderr. Most of the modules: canbusload.c,
> > candump.c and canlogserver.c use stdout but
> > mcp251xfd/mcp251xfd-dev-coredump.c uses stderr. Harmonize the behavior
> > by following the major trend and make
> > mcp251xfd/mcp251xfd-dev-coredump.c also output to stdout.
> >
> > CC: Marc Kleine-Budde <mkl@pengutronix.de>
> > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> > ---
> > @Marc, was there any reasons to print debug information to stderr and
> > not stdout in mcp251xfd-dev-coredump.c?
>
> No real reason, just gut feeling. There are at least 2 differences: IIRC
> stdout is line buffered, it will not write to console until a newline.
> stderr will print even if there is no newline.

Something I learned today!

I thought that all the buffering was done by the libc (i.e.
write(stdout, "foo") is not buffered while fprint(stdout, "foo") is.
But after investigation, you are right. Indeed, 'man 3 stdout' says:

  The stream stderr is unbuffered.  The stream stdout is
  line-buffered when it points to a terminal.  Partial lines will
  not appear until fflush(3) or exit(3) is called, or a newline
  is printed.  This can produce unexpected results, especially
  with debugging output.

> The other one is: If
> use/parse the stdout if the program you're debugging (i.e. in a pipe)
> the debug output on stdout will interfere with the regular output.

I agree. I am fine with putting the debug to stderr or to prefix the
stdout output with "debug: " or something similar so that it can be
easily filtered (usually, I do the latter). The only things which
really bothered me was the #DEBUG everywhere and the inconsistency of
stdout vs. stderr. As long as those things are fixed, anything else is
OK for me.

By default, I just did the most concervative change: everyone prints
to stdout and I did not add a prefix.

> > diff --git a/lib.h b/lib.h
> > index a4d3ce5..1cb1dd4 100644
> > --- a/lib.h
> > +++ b/lib.h
> > @@ -47,6 +47,12 @@
> >
> >  #include <stdio.h>
> >
> > +#ifdef DEBUG
> > +#define pr_debug(fmt, args...) printf(fmt, ##args)
> > +#else
> > +#define pr_debug(...)
> > +#endif
>
> With this change if DEBUG is not defined, the print format strings are
> not checked. This is why I have the pr_no macro. Side node: For functions
> you can use __attribute__ ((format (printf, 2, 3))).

I never thought of that. Now I understand why you put a pr_no macro.
I knew about the print attribute and I prefer to use it instead of the
pr_no to make it more explicit that this is only for compiler check.
Now, I am thinking to do:

  #ifdef DEBUG
  #define pr_debug(fmt, args...) printf(fmt, ##args)
  #else
  __attribute__((format (printf, 1, 2)))
  static inline int pr_debug(const char* fmt, ...) {return 0;}
  #endif

> > +
> >  /* buffer sizes for CAN frame string representations */
> >
> >  #define CL_ID (sizeof("12345678##1"))
> > diff --git a/mcp251xfd/mcp251xfd-dev-coredump.c b/mcp251xfd/mcp251xfd-dev-coredump.c
> > index 5874d24..422900f 100644
> > --- a/mcp251xfd/mcp251xfd-dev-coredump.c
> > +++ b/mcp251xfd/mcp251xfd-dev-coredump.c
> > @@ -17,18 +17,10 @@
> >
> >  #include <linux/kernel.h>
> >
> > +#include "../lib.h"
> >  #include "mcp251xfd.h"
> >  #include "mcp251xfd-dump-userspace.h"
> >
> > -#define pr_err(fmt, args...)    fprintf(stderr, fmt, ##args)
> > -#define pr_no(fmt, args...)     while (0) { fprintf(stdout, fmt, ##args); }
> > -
> > -#ifdef DEBUG
> > -#define pr_debug(fmt, args...) pr_err(fmt, ##args)
> > -#else
> > -#define pr_debug(fmt, args...) pr_no(fmt, ##args)
> > -#endif
> > -
> >
> >  struct mcp251xfd_dump_iter {
> >       const void *start;
> > diff --git a/slcanpty.c b/slcanpty.c
> > index 4ac9e8a..3eba07e 100644
> > --- a/slcanpty.c
> > +++ b/slcanpty.c
> > @@ -49,8 +49,6 @@
> >  #define SLC_MTU (sizeof("T1111222281122334455667788EA5F\r")+1)
> >  #define DEVICE_NAME_PTMX "/dev/ptmx"
> >
> > -#define DEBUG
>
> For completeness mention that you switch off debugging in slcanpty.c.

Ack.


Yours sincerely,
Vincent Mailhol

Re: [PATCH can-utils-dev 2/5] lib: add pr_debug() macro

[Marc Kleine-Budde]

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

On 13.11.2022 17:53:18, Vincent Mailhol wrote:

> diff --git a/mcp251xfd/mcp251xfd-dev-coredump.c b/mcp251xfd/mcp251xfd-dev-coredump.c
> index 5874d24..422900f 100644
> --- a/mcp251xfd/mcp251xfd-dev-coredump.c
> +++ b/mcp251xfd/mcp251xfd-dev-coredump.c
> @@ -17,18 +17,10 @@
>  
>  #include <linux/kernel.h>
>  
> +#include "../lib.h"

Does #include <lib.h> work?

>  #include "mcp251xfd.h"
>  #include "mcp251xfd-dump-userspace.h"
>  
> -#define pr_err(fmt, args...)    fprintf(stderr, fmt, ##args)
> -#define pr_no(fmt, args...)     while (0) { fprintf(stdout, fmt, ##args); }
> -
> -#ifdef DEBUG
> -#define pr_debug(fmt, args...) pr_err(fmt, ##args)
> -#else
> -#define pr_debug(fmt, args...) pr_no(fmt, ##args)
> -#endif
> -
>  
>  struct mcp251xfd_dump_iter {
>  	const void *start;

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

Re: [PATCH can-utils-dev 2/5] lib: add pr_debug() macro

[Vincent MAILHOL]

On Sun. 13 Nov. 2022 at 22:12, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 13.11.2022 17:53:18, Vincent Mailhol wrote:
>
> > diff --git a/mcp251xfd/mcp251xfd-dev-coredump.c b/mcp251xfd/mcp251xfd-dev-coredump.c
> > index 5874d24..422900f 100644
> > --- a/mcp251xfd/mcp251xfd-dev-coredump.c
> > +++ b/mcp251xfd/mcp251xfd-dev-coredump.c
> > @@ -17,18 +17,10 @@
> >
> >  #include <linux/kernel.h>
> >
> > +#include "../lib.h"
>
> Does #include <lib.h> work?

Unfortunately, no:

  cc -O2 -Wall -Wno-parentheses -Iinclude -DAF_CAN=PF_CAN -DPF_CAN=29
-DSO_RXQ_OVFL=40 -DSCM_TIMESTAMPING_OPT_STATS=54
-D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE  -c -o
mcp251xfd/mcp251xfd-dev-coredump.o mcp251xfd/mcp251xfd-dev-coredump.c
  mcp251xfd/mcp251xfd-dev-coredump.c:20:10: fatal error: lib.h: No
such file or directory
     20 | #include <lib.h>
        |          ^~~~~~~
  compilation terminated.
  make: *** [<builtin>: mcp251xfd/mcp251xfd-dev-coredump.o] Error 1

> >  #include "mcp251xfd.h"
> >  #include "mcp251xfd-dump-userspace.h"
> >
> > -#define pr_err(fmt, args...)    fprintf(stderr, fmt, ##args)
> > -#define pr_no(fmt, args...)     while (0) { fprintf(stdout, fmt, ##args); }
> > -
> > -#ifdef DEBUG
> > -#define pr_debug(fmt, args...) pr_err(fmt, ##args)
> > -#else
> > -#define pr_debug(fmt, args...) pr_no(fmt, ##args)
> > -#endif
> > -
> >
> >  struct mcp251xfd_dump_iter {
> >       const void *start;
>
> 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 |

Re: [PATCH can-utils-dev 2/5] lib: add pr_debug() macro

[Marc Kleine-Budde]

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

On 13.11.2022 22:19:23, Vincent MAILHOL wrote:
> On Sun. 13 Nov. 2022 at 22:12, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> > On 13.11.2022 17:53:18, Vincent Mailhol wrote:
> >
> > > diff --git a/mcp251xfd/mcp251xfd-dev-coredump.c b/mcp251xfd/mcp251xfd-dev-coredump.c
> > > index 5874d24..422900f 100644
> > > --- a/mcp251xfd/mcp251xfd-dev-coredump.c
> > > +++ b/mcp251xfd/mcp251xfd-dev-coredump.c
> > > @@ -17,18 +17,10 @@
> > >
> > >  #include <linux/kernel.h>
> > >
> > > +#include "../lib.h"
> >
> > Does #include <lib.h> work?
> 
> Unfortunately, no:

Using the plain makefile without autotools and cmake?

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: 484 bytes --]

Re: [PATCH can-utils-dev 2/5] lib: add pr_debug() macro

[Vincent MAILHOL]

On Sun. 13 nov. 2022 at 22:39, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 13.11.2022 22:19:23, Vincent MAILHOL wrote:
> > On Sun. 13 Nov. 2022 at 22:12, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> > > On 13.11.2022 17:53:18, Vincent Mailhol wrote:
> > >
> > > > diff --git a/mcp251xfd/mcp251xfd-dev-coredump.c b/mcp251xfd/mcp251xfd-dev-coredump.c
> > > > index 5874d24..422900f 100644
> > > > --- a/mcp251xfd/mcp251xfd-dev-coredump.c
> > > > +++ b/mcp251xfd/mcp251xfd-dev-coredump.c
> > > > @@ -17,18 +17,10 @@
> > > >
> > > >  #include <linux/kernel.h>
> > > >
> > > > +#include "../lib.h"
> > >
> > > Does #include <lib.h> work?
> >
> > Unfortunately, no:
>
> Using the plain makefile without autotools and cmake?

It works with autotools. Actually, I am now confused, I thought that
Makefile would be generated by ./autogen.sh and ./configure. To be
honest, after seeing the Makefile, I did not pay attention to the rest
and did not realize that this was also a autotools project.
Shouldn't the Makefile be removed? It is odd to have both.

Re: [PATCH can-utils-dev 2/5] lib: add pr_debug() macro

[Marc Kleine-Budde]

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

On 13.11.2022 22:47:14, Vincent MAILHOL wrote:
> On Sun. 13 nov. 2022 at 22:39, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> > On 13.11.2022 22:19:23, Vincent MAILHOL wrote:
> > > On Sun. 13 Nov. 2022 at 22:12, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> > > > On 13.11.2022 17:53:18, Vincent Mailhol wrote:
> > > >
> > > > > diff --git a/mcp251xfd/mcp251xfd-dev-coredump.c b/mcp251xfd/mcp251xfd-dev-coredump.c
> > > > > index 5874d24..422900f 100644
> > > > > --- a/mcp251xfd/mcp251xfd-dev-coredump.c
> > > > > +++ b/mcp251xfd/mcp251xfd-dev-coredump.c
> > > > > @@ -17,18 +17,10 @@
> > > > >
> > > > >  #include <linux/kernel.h>
> > > > >
> > > > > +#include "../lib.h"
> > > >
> > > > Does #include <lib.h> work?
> > >
> > > Unfortunately, no:
> >
> > Using the plain makefile without autotools and cmake?
> 
> It works with autotools. Actually, I am now confused, I thought that
> Makefile would be generated by ./autogen.sh and ./configure. To be
> honest, after seeing the Makefile, I did not pay attention to the rest
> and did not realize that this was also a autotools project.
> Shouldn't the Makefile be removed? It is odd to have both.

I don't see any benefits in the handcrafted Makefile, Oliver seems to
like it :)

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

Re: [PATCH can-utils-dev 2/5] lib: add pr_debug() macro

[Vincent MAILHOL]

On Sun. 13 Nov. 2022 at 23:04, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> On 13.11.2022 22:47:14, Vincent MAILHOL wrote:
> > On Sun. 13 nov. 2022 at 22:39, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> > > On 13.11.2022 22:19:23, Vincent MAILHOL wrote:
> > > > On Sun. 13 Nov. 2022 at 22:12, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> > > > > On 13.11.2022 17:53:18, Vincent Mailhol wrote:
> > > > >
> > > > > > diff --git a/mcp251xfd/mcp251xfd-dev-coredump.c b/mcp251xfd/mcp251xfd-dev-coredump.c
> > > > > > index 5874d24..422900f 100644
> > > > > > --- a/mcp251xfd/mcp251xfd-dev-coredump.c
> > > > > > +++ b/mcp251xfd/mcp251xfd-dev-coredump.c
> > > > > > @@ -17,18 +17,10 @@
> > > > > >
> > > > > >  #include <linux/kernel.h>
> > > > > >
> > > > > > +#include "../lib.h"
> > > > >
> > > > > Does #include <lib.h> work?
> > > >
> > > > Unfortunately, no:
> > >
> > > Using the plain makefile without autotools and cmake?
> >
> > It works with autotools. Actually, I am now confused, I thought that
> > Makefile would be generated by ./autogen.sh and ./configure. To be
> > honest, after seeing the Makefile, I did not pay attention to the rest
> > and did not realize that this was also a autotools project.
> > Shouldn't the Makefile be removed? It is odd to have both.
>
> I don't see any benefits in the handcrafted Makefile, Oliver seems to
> like it :)

Well, I think it should be either, not both. And can-utils being
popular and included in many distro such as Debian, I think it is
better to keep the autotools. But I am not going to change it myself.

This also means that it will be #include "../lib.h" and not #include
<lib.h> (unless the handcrafted Makefile gets a header include
directive).

Unless you have a last comment, I will send the v2 in a couple of minutes.


Yours sincerely,
Vincent Mailhol

[PATCH can-utils-dev 3/5] candump: add global variable progname

[Vincent Mailhol]

Instead of using argv[0] several time, make the progname a global
variable.

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 candump.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/candump.c b/candump.c
index 4a239e7..4abd2f6 100644
--- a/candump.c
+++ b/candump.c
@@ -108,6 +108,7 @@ struct if_info { /* bundled information per open socket */
 };
 static struct if_info sock_info[MAXSOCK];
 
+static char *progname;
 static char devname[MAXIFNAMES][IFNAMSIZ+1];
 static int dindex[MAXIFNAMES];
 static int max_devname_len; /* to prevent frazzled device name output */
@@ -121,11 +122,11 @@ extern int optind, opterr, optopt;
 
 static volatile int running = 1;
 
-static void print_usage(char *prg)
+static void print_usage(void)
 {
-	fprintf(stderr, "%s - dump CAN bus traffic.\n", prg);
-	fprintf(stderr, "\nUsage: %s [options] <CAN interface>+\n", prg);
-	fprintf(stderr, "  (use CTRL-C to terminate %s)\n\n", prg);
+	fprintf(stderr, "%s - dump CAN bus traffic.\n", progname);
+	fprintf(stderr, "\nUsage: %s [options] <CAN interface>+\n", progname);
+	fprintf(stderr, "  (use CTRL-C to terminate %s)\n\n", progname);
 	fprintf(stderr, "Options:\n");
 	fprintf(stderr, "         -t <type>   (timestamp: (a)bsolute/(d)elta/(z)ero/(A)bsolute w date)\n");
 	fprintf(stderr, "         -H          (read hardware timestamps instead of system timestamps)\n");
@@ -159,12 +160,12 @@ static void print_usage(char *prg)
 	fprintf(stderr, "Without any given filter all data frames are received ('0:0' default filter).\n");
 	fprintf(stderr, "\nUse interface name '%s' to receive from all CAN interfaces.\n", ANYDEV);
 	fprintf(stderr, "\nExamples:\n");
-	fprintf(stderr, "%s -c -c -ta can0,123:7FF,400:700,#000000FF can2,400~7F0 can3 can8\n\n", prg);
-	fprintf(stderr, "%s -l any,0~0,#FFFFFFFF\n         (log only error frames but no(!) data frames)\n", prg);
-	fprintf(stderr, "%s -l any,0:0,#FFFFFFFF\n         (log error frames and also all data frames)\n", prg);
-	fprintf(stderr, "%s vcan2,12345678:DFFFFFFF\n         (match only for extended CAN ID 12345678)\n", prg);
-	fprintf(stderr, "%s vcan2,123:7FF\n         (matches CAN ID 123 - including EFF and RTR frames)\n", prg);
-	fprintf(stderr, "%s vcan2,123:C00007FF\n         (matches CAN ID 123 - only SFF and non-RTR frames)\n", prg);
+	fprintf(stderr, "%s -c -c -ta can0,123:7FF,400:700,#000000FF can2,400~7F0 can3 can8\n\n", progname);
+	fprintf(stderr, "%s -l any,0~0,#FFFFFFFF\n         (log only error frames but no(!) data frames)\n", progname);
+	fprintf(stderr, "%s -l any,0:0,#FFFFFFFF\n         (log error frames and also all data frames)\n", progname);
+	fprintf(stderr, "%s vcan2,12345678:DFFFFFFF\n         (match only for extended CAN ID 12345678)\n", progname);
+	fprintf(stderr, "%s vcan2,123:7FF\n         (matches CAN ID 123 - including EFF and RTR frames)\n", progname);
+	fprintf(stderr, "%s vcan2,123:C00007FF\n         (matches CAN ID 123 - only SFF and non-RTR frames)\n", progname);
 	fprintf(stderr, "\n");
 }
 
@@ -322,6 +323,8 @@ int main(int argc, char **argv)
 	last_tv.tv_sec = 0;
 	last_tv.tv_usec = 0;
 
+	progname = basename(argv[0]);
+
 	while ((opt = getopt(argc, argv, "t:HciaSs:lDdxLf:n:r:he8T:?")) != -1) {
 		switch (opt) {
 		case 't':
@@ -330,7 +333,7 @@ int main(int argc, char **argv)
 			if ((timestamp != 'a') && (timestamp != 'A') &&
 			    (timestamp != 'd') && (timestamp != 'z')) {
 				fprintf(stderr, "%s: unknown timestamp mode '%c' - ignored\n",
-					basename(argv[0]), optarg[0]);
+					progname, optarg[0]);
 				timestamp = 0;
 			}
 			if ((logtimestamp != 'a') && (logtimestamp != 'z')) {
@@ -369,7 +372,7 @@ int main(int argc, char **argv)
 		case 's':
 			silent = atoi(optarg);
 			if (silent > SILENT_ON) {
-				print_usage(basename(argv[0]));
+				print_usage();
 				exit(1);
 			}
 			break;
@@ -402,7 +405,7 @@ int main(int argc, char **argv)
 		case 'n':
 			count = atoi(optarg);
 			if (count < 1) {
-				print_usage(basename(argv[0]));
+				print_usage();
 				exit(1);
 			}
 			break;
@@ -410,7 +413,7 @@ int main(int argc, char **argv)
 		case 'r':
 			rcvbuf_size = atoi(optarg);
 			if (rcvbuf_size < 1) {
-				print_usage(basename(argv[0]));
+				print_usage();
 				exit(1);
 			}
 			break;
@@ -419,19 +422,19 @@ int main(int argc, char **argv)
 			errno = 0;
 			timeout_ms = strtol(optarg, NULL, 0);
 			if (errno != 0) {
-				print_usage(basename(argv[0]));
+				print_usage();
 				exit(1);
 			}
 			break;
 		default:
-			print_usage(basename(argv[0]));
+			print_usage();
 			exit(1);
 			break;
 		}
 	}
 
 	if (optind == argc) {
-		print_usage(basename(argv[0]));
+		print_usage();
 		exit(0);
 	}
 
-- 
2.37.4

[PATCH can-utils-dev 4/5] candump: use linux/net_tstamp.h instead of redefining values ourselves

[Vincent Mailhol]

No need to redefine values when available in header.

linux/net_tstamp.h is available since Linux 2.6.30 while socket CAN
was introduced in v2.6.25. These being old releases not being
maintained any more for many years, dropping support is
acceptable.

Regardless, candump already relies on some other macros defined in
more recent kernel version (e.g. CAN_RAW_ERR_FILTER) meaning that it
would not build on old linux kernel environments.

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 candump.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/candump.c b/candump.c
index 4abd2f6..8f0ca46 100644
--- a/candump.c
+++ b/candump.c
@@ -63,6 +63,7 @@
 
 #include <linux/can.h>
 #include <linux/can/raw.h>
+#include <linux/net_tstamp.h>
 
 #include "terminal.h"
 #include "lib.h"
@@ -72,10 +73,6 @@
 #define SO_TIMESTAMPING 37
 #endif
 
-/* from #include <linux/net_tstamp.h> - since Linux 2.6.30 */
-#define SOF_TIMESTAMPING_SOFTWARE (1 << 4)
-#define SOF_TIMESTAMPING_RX_SOFTWARE (1 << 3)
-#define SOF_TIMESTAMPING_RAW_HARDWARE (1 << 6)
 #define TIMESTAMPSZ 50 /* string 'absolute with date' requires max 49 bytes */
 
 #define MAXSOCK 16    /* max. number of CAN interfaces given on the cmdline */
-- 
2.37.4

Re: [PATCH can-utils-dev 4/5] candump: use linux/net_tstamp.h instead of redefining values ourselves

[Oliver Hartkopp]

On 13.11.22 09:53, Vincent Mailhol wrote:
> No need to redefine values when available in header.
> 
> linux/net_tstamp.h is available since Linux 2.6.30 while socket CAN
> was introduced in v2.6.25. These being old releases not being
> maintained any more for many years, dropping support is
> acceptable.
> 
> Regardless, candump already relies on some other macros defined in
> more recent kernel version (e.g. CAN_RAW_ERR_FILTER) meaning that it
> would not build on old linux kernel environments.

The patch is right but this text does not fit IMO.

We have a copy of net_tstamp.h in this repository to make sure we can 
always build the latest binaries with the latest kernel APIs even on 
older development environments/kernels.

Best regards,
Oliver

> 
> Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> ---
>   candump.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/candump.c b/candump.c
> index 4abd2f6..8f0ca46 100644
> --- a/candump.c
> +++ b/candump.c
> @@ -63,6 +63,7 @@
>   
>   #include <linux/can.h>
>   #include <linux/can/raw.h>
> +#include <linux/net_tstamp.h>
>   
>   #include "terminal.h"
>   #include "lib.h"
> @@ -72,10 +73,6 @@
>   #define SO_TIMESTAMPING 37
>   #endif
>   
> -/* from #include <linux/net_tstamp.h> - since Linux 2.6.30 */
> -#define SOF_TIMESTAMPING_SOFTWARE (1 << 4)
> -#define SOF_TIMESTAMPING_RX_SOFTWARE (1 << 3)
> -#define SOF_TIMESTAMPING_RAW_HARDWARE (1 << 6)
>   #define TIMESTAMPSZ 50 /* string 'absolute with date' requires max 49 bytes */
>   
>   #define MAXSOCK 16    /* max. number of CAN interfaces given on the cmdline */

Re: [PATCH can-utils-dev 4/5] candump: use linux/net_tstamp.h instead of redefining values ourselves

[Vincent MAILHOL]

On Mon. 14 Nov. 2022 at 05:05, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> On 13.11.22 09:53, Vincent Mailhol wrote:
> > No need to redefine values when available in header.
> >
> > linux/net_tstamp.h is available since Linux 2.6.30 while socket CAN
> > was introduced in v2.6.25. These being old releases not being
> > maintained any more for many years, dropping support is
> > acceptable.
> >
> > Regardless, candump already relies on some other macros defined in
> > more recent kernel version (e.g. CAN_RAW_ERR_FILTER) meaning that it
> > would not build on old linux kernel environments.
>
> The patch is right but this text does not fit IMO.
>
> We have a copy of net_tstamp.h in this repository to make sure we can
> always build the latest binaries with the latest kernel APIs even on
> older development environments/kernels.

You are right. I missed the fact that there was a local copy of the
kernel headers at the include/linux/ directory at the root of the
project.

I will amend the description. I will also give a second thought on
patch 5/5: "lib: snprintf_can_error_frame: print counter errors if
CAN_ERR_CNT is set" as this one might have repercussions if built on a
new machine and run on an older one.


Yours sincerely,
Vincent Mailhol

Re: [PATCH can-utils-dev 4/5] candump: use linux/net_tstamp.h instead of redefining values ourselves

[Oliver Hartkopp]

On 14.11.22 06:25, Vincent MAILHOL wrote:
> On Mon. 14 Nov. 2022 at 05:05, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>> On 13.11.22 09:53, Vincent Mailhol wrote:
>>> No need to redefine values when available in header.
>>>
>>> linux/net_tstamp.h is available since Linux 2.6.30 while socket CAN
>>> was introduced in v2.6.25. These being old releases not being
>>> maintained any more for many years, dropping support is
>>> acceptable.
>>>
>>> Regardless, candump already relies on some other macros defined in
>>> more recent kernel version (e.g. CAN_RAW_ERR_FILTER) meaning that it
>>> would not build on old linux kernel environments.
>>
>> The patch is right but this text does not fit IMO.
>>
>> We have a copy of net_tstamp.h in this repository to make sure we can
>> always build the latest binaries with the latest kernel APIs even on
>> older development environments/kernels.
> 
> You are right. I missed the fact that there was a local copy of the
> kernel headers at the include/linux/ directory at the root of the
> project.
> 
> I will amend the description. I will also give a second thought on
> patch 5/5: "lib: snprintf_can_error_frame: print counter errors if
> CAN_ERR_CNT is set" as this one might have repercussions if built on a
> new machine and run on an older one.

Good idea!

Thanks!

Oliver

[PATCH can-utils-dev 5/5] lib: snprintf_can_error_frame: print counter errors if CAN_ERR_CNT is set

[Vincent Mailhol]

Since [1], the kernel has a new flag: CAN_ERR_CNT to notify whether or
not the errour counter is set. Use this to decide whether on not the
error-counter-tx-rx should be printed.

For interoperability reasons, use an #ifdef so that the code still
work on older kernels.

[1] commit 3e5c291c7942 ("can: add CAN_ERR_CNT flag to notify
    availability of error counter")
Link: https://git.kernel.org/torvalds/linux/c/3e5c291c7942

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 lib.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib.c b/lib.c
index 3c1a0d9..fb08c0b 100644
--- a/lib.c
+++ b/lib.c
@@ -679,7 +679,11 @@ void snprintf_can_error_frame(char *buf, size_t len, const struct canfd_frame *c
 		}
 	}
 
+#ifdef CAN_ERR_CNT
+	if (cf->can_id & CAN_ERR_CNT) {
+#else
 	if (cf->data[6] || cf->data[7]) {
+#endif
 		n += snprintf(buf + n, len - n, "%s", sep);
 		n += snprintf(buf + n, len - n, "error-counter-tx-rx{{%d}{%d}}",
 			      cf->data[6], cf->data[7]);
-- 
2.37.4

Re: [PATCH can-utils-dev 0/5] can-utils-dev: a set of cleanup patches

[Marc Kleine-Budde]

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

On 13.11.2022 17:53:16, Vincent Mailhol wrote:
> This series contain five cleanup patches. There is no real relation
> between each patch so I will end the cover letter here and let you
> refer to the short description.

See my comment on patch 2, the rest looks good.

Marc

P.S.: You can either send a PR on github, or a v2 via mail here.

-- 
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-utils-dev v2 0/5] can-utils-dev: a set of cleanup patches

[Vincent Mailhol]

This series contain five cleanup patches. There is no real relation
between each patch so I will end the cover letter here and let you
refer to the short description.

* Changelog *

v1 -> v2:

  * patch 2/5: add a note in commit message to explain that slcanpty.c
    debug is deactivated by default.

  * patch 2/5: make the compiler check the pr_debug() syntax even if
    debug is off.

  * reorder the patches so that the lib.c and the candump.c patches
    are next to each other in the series.

Vincent Mailhol (5):
  slcanpty: remove redundant asc2nibble()
  lib: add pr_debug() macro
  lib: snprintf_can_error_frame: print counter errors if CAN_ERR_CNT is
    set
  candump: add global variable progname
  candump: use linux/net_tstamp.h instead of redefining values ourselves

 Makefile                           |  2 ++
 canbusload.c                       |  9 ++---
 candump.c                          | 58 +++++++++++++-----------------
 canlogserver.c                     | 12 +++----
 lib.c                              |  4 +++
 lib.h                              |  7 ++++
 mcp251xfd/mcp251xfd-dev-coredump.c | 10 +-----
 slcanpty.c                         | 27 +++-----------
 8 files changed, 51 insertions(+), 78 deletions(-)

-- 
2.37.4

[PATCH can-utils-dev v2 1/5] slcanpty: remove redundant asc2nibble()

[Vincent Mailhol]

asc2nibble() is already defined in lib.h. Include lib.h in slcnpty.c
so that asc2nibble() does not need to be redefined a second time and
adjust the Makefile accordingly.

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 Makefile   |  2 ++
 slcanpty.c | 17 ++---------------
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index fb52643..cb67b66 100644
--- a/Makefile
+++ b/Makefile
@@ -119,6 +119,7 @@ canplayer.o:	lib.h
 cansend.o:	lib.h
 log2asc.o:	lib.h
 log2long.o:	lib.h
+slcanpty.o:	lib.h
 j1939acd.o:	libj1939.h
 j1939cat.o:	libj1939.h
 j1939spy.o:	libj1939.h
@@ -135,6 +136,7 @@ cansend:	cansend.o	lib.o
 cansequence:	cansequence.o	lib.o
 log2asc:	log2asc.o	lib.o
 log2long:	log2long.o	lib.o
+slcanpty:	slcanpty.o	lib.o
 j1939acd:	j1939acd.o	libj1939.o
 j1939cat:	j1939cat.o	libj1939.o
 j1939spy:	j1939spy.o	libj1939.o
diff --git a/slcanpty.c b/slcanpty.c
index e6f1efe..4ac9e8a 100644
--- a/slcanpty.c
+++ b/slcanpty.c
@@ -43,27 +43,14 @@
 #include <linux/can/raw.h>
 #include <linux/sockios.h>
 
+#include "lib.h"
+
 /* maximum rx buffer len: extended CAN frame with timestamp */
 #define SLC_MTU (sizeof("T1111222281122334455667788EA5F\r")+1)
 #define DEVICE_NAME_PTMX "/dev/ptmx"
 
 #define DEBUG
 
-static int asc2nibble(char c)
-{
-
-	if ((c >= '0') && (c <= '9'))
-		return c - '0';
-
-	if ((c >= 'A') && (c <= 'F'))
-		return c - 'A' + 10;
-
-	if ((c >= 'a') && (c <= 'f'))
-		return c - 'a' + 10;
-
-	return 16; /* error */
-}
-
 /* read data from pty, send CAN frames to CAN socket and answer commands */
 int pty2can(int pty, int socket, struct can_filter *fi,
 	    int *is_open, int *tstamp)
-- 
2.37.4

[PATCH can-utils-dev v2 2/5] lib: add pr_debug() macro

[Vincent Mailhol]

Add the pr_debug() macro so that:

  #ifdef DEBUG
  	printf("foo");
  #endif

can be replaced by:

  	pr_debug("foo");

Apply the pr_debug() macro wherever relevant.

Currently, there is no consensus whether debug messages should be
printed on stdout or stderr. Most of the modules: canbusload.c,
candump.c and canlogserver.c use stdout but
mcp251xfd/mcp251xfd-dev-coredump.c uses stderr. Harmonize the behavior
by following the major trend and make
mcp251xfd/mcp251xfd-dev-coredump.c also output to stdout.

slcanpty.c does a #define DEBUG, meaning that debug is always turned
on for this file. Remove this and make debug an option like every
other files.

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 canbusload.c                       |  9 +++------
 candump.c                          | 16 ++++------------
 canlogserver.c                     | 12 ++++--------
 lib.h                              |  7 +++++++
 mcp251xfd/mcp251xfd-dev-coredump.c | 10 +---------
 slcanpty.c                         | 10 +++-------
 6 files changed, 22 insertions(+), 42 deletions(-)

diff --git a/canbusload.c b/canbusload.c
index e4dfc02..47b62fd 100644
--- a/canbusload.c
+++ b/canbusload.c
@@ -61,6 +61,7 @@
 #include <linux/can.h>
 #include <linux/can/raw.h>
 
+#include "lib.h"
 #include "terminal.h"
 #include "canframelen.h"
 
@@ -310,9 +311,7 @@ int main(int argc, char **argv)
 			return 1;
 		}
 
-#ifdef DEBUG
-		printf("open %d '%s'.\n", i, ptr);
-#endif
+		pr_debug("open %d '%s'.\n", i, ptr);
 
 		s[i] = socket(PF_CAN, SOCK_RAW, CAN_RAW);
 		if (s[i] < 0) {
@@ -358,10 +357,8 @@ int main(int argc, char **argv)
 		if (nbytes > max_bitrate_len)
 			max_bitrate_len = nbytes; /* for nice printing */
 
+		pr_debug("using interface name '%s'.\n", ifr.ifr_name);
 
-#ifdef DEBUG
-		printf("using interface name '%s'.\n", ifr.ifr_name);
-#endif
 		/* try to switch the socket into CAN FD mode */
 		const int canfd_on = 1;
 		setsockopt(s[i], SOL_CAN_RAW, CAN_RAW_FD_FRAMES, &canfd_on, sizeof(canfd_on));
diff --git a/candump.c b/candump.c
index 6fe30bb..4a239e7 100644
--- a/candump.c
+++ b/candump.c
@@ -216,9 +216,7 @@ static int idx2dindex(int ifidx, int socket)
 
 	strcpy(devname[i], ifr.ifr_name);
 
-#ifdef DEBUG
-	printf("new index %d (%s)\n", i, devname[i]);
-#endif
+	pr_debug("new index %d (%s)\n", i, devname[i]);
 
 	return i;
 }
@@ -474,9 +472,7 @@ int main(int argc, char **argv)
 		ptr = argv[optind+i];
 		nptr = strchr(ptr, ',');
 
-#ifdef DEBUG
-		printf("open %d '%s'.\n", i, ptr);
-#endif
+		pr_debug("open %d '%s'.\n", i, ptr);
 
 		obj->s = socket(PF_CAN, SOCK_RAW, CAN_RAW);
 		if (obj->s < 0) {
@@ -510,9 +506,7 @@ int main(int argc, char **argv)
 		memset(&ifr.ifr_name, 0, sizeof(ifr.ifr_name));
 		strncpy(ifr.ifr_name, ptr, nbytes);
 
-#ifdef DEBUG
-		printf("using interface name '%s'.\n", ifr.ifr_name);
-#endif
+		pr_debug("using interface name '%s'.\n", ifr.ifr_name);
 
 		if (strcmp(ANYDEV, ifr.ifr_name) != 0) {
 			if (ioctl(obj->s, SIOCGIFINDEX, &ifr) < 0) {
@@ -602,9 +596,7 @@ int main(int argc, char **argv)
 			/* try SO_RCVBUFFORCE first, if we run with CAP_NET_ADMIN */
 			if (setsockopt(obj->s, SOL_SOCKET, SO_RCVBUFFORCE,
 				       &rcvbuf_size, sizeof(rcvbuf_size)) < 0) {
-#ifdef DEBUG
-				printf("SO_RCVBUFFORCE failed so try SO_RCVBUF ...\n");
-#endif
+				pr_debug("SO_RCVBUFFORCE failed so try SO_RCVBUF ...\n");
 				if (setsockopt(obj->s, SOL_SOCKET, SO_RCVBUF,
 					       &rcvbuf_size, sizeof(rcvbuf_size)) < 0) {
 					perror("setsockopt SO_RCVBUF");
diff --git a/canlogserver.c b/canlogserver.c
index 6425ca4..51d548f 100644
--- a/canlogserver.c
+++ b/canlogserver.c
@@ -145,9 +145,7 @@ int idx2dindex(int ifidx, int socket)
 
 	strcpy(devname[i], ifr.ifr_name);
 
-#ifdef DEBUG
-	printf("new index %d (%s)\n", i, devname[i]);
-#endif
+	pr_debug("new index %d (%s)\n", i, devname[i]);
 
 	return i;
 }
@@ -310,11 +308,9 @@ int main(int argc, char **argv)
 
 	for (i=0; i<currmax; i++) {
 
-#ifdef DEBUG
-		printf("open %d '%s' m%08X v%08X i%d e%d.\n",
-		       i, argv[optind+i], mask[i], value[i],
-		       inv_filter[i], err_mask[i]);
-#endif
+		pr_debug("open %d '%s' m%08X v%08X i%d e%d.\n",
+		      i, argv[optind+i], mask[i], value[i],
+		      inv_filter[i], err_mask[i]);
 
 		if ((s[i] = socket(PF_CAN, SOCK_RAW, CAN_RAW)) < 0) {
 			perror("socket");
diff --git a/lib.h b/lib.h
index a4d3ce5..6cc58a2 100644
--- a/lib.h
+++ b/lib.h
@@ -47,6 +47,13 @@
 
 #include <stdio.h>
 
+#ifdef DEBUG
+#define pr_debug(fmt, args...) printf(fmt, ##args)
+#else
+__attribute__((format (printf, 1, 2)))
+static inline int pr_debug(const char* fmt, ...) {return 0;}
+#endif
+
 /* buffer sizes for CAN frame string representations */
 
 #define CL_ID (sizeof("12345678##1"))
diff --git a/mcp251xfd/mcp251xfd-dev-coredump.c b/mcp251xfd/mcp251xfd-dev-coredump.c
index 5874d24..422900f 100644
--- a/mcp251xfd/mcp251xfd-dev-coredump.c
+++ b/mcp251xfd/mcp251xfd-dev-coredump.c
@@ -17,18 +17,10 @@
 
 #include <linux/kernel.h>
 
+#include "../lib.h"
 #include "mcp251xfd.h"
 #include "mcp251xfd-dump-userspace.h"
 
-#define pr_err(fmt, args...)    fprintf(stderr, fmt, ##args)
-#define pr_no(fmt, args...)     while (0) { fprintf(stdout, fmt, ##args); }
-
-#ifdef DEBUG
-#define pr_debug(fmt, args...) pr_err(fmt, ##args)
-#else
-#define pr_debug(fmt, args...) pr_no(fmt, ##args)
-#endif
-
 
 struct mcp251xfd_dump_iter {
 	const void *start;
diff --git a/slcanpty.c b/slcanpty.c
index 4ac9e8a..fa97cd6 100644
--- a/slcanpty.c
+++ b/slcanpty.c
@@ -49,8 +49,6 @@
 #define SLC_MTU (sizeof("T1111222281122334455667788EA5F\r")+1)
 #define DEVICE_NAME_PTMX "/dev/ptmx"
 
-#define DEBUG
-
 /* read data from pty, send CAN frames to CAN socket and answer commands */
 int pty2can(int pty, int socket, struct can_filter *fi,
 	    int *is_open, int *tstamp)
@@ -106,14 +104,12 @@ rx_restart:
 	cmd = buf[0];
 	buf[nbytes] = 0;
 
-#ifdef DEBUG
 	for (tmp = 0; tmp < nbytes; tmp++)
 		if (buf[tmp] == '\r')
-			putchar('@');
+			pr_debug("@");
 		else
-			putchar(buf[tmp]);
-	printf("\n");
-#endif
+			pr_debug("%c", buf[tmp]);
+	pr_debug("\n");
 
 	/* check for filter configuration commands */
 	if (cmd == 'm' || cmd == 'M') {
-- 
2.37.4

[PATCH can-utils-dev v2 3/5] lib: snprintf_can_error_frame: print counter errors if CAN_ERR_CNT is set

[Vincent Mailhol]

Since [1], the kernel has a new flag: CAN_ERR_CNT to notify whether or
not the error counter is set. Use this to decide whether on not the
error-counter-tx-rx should be printed.

For interoperability reasons, use an #ifdef so that the code still
work if built on older kernels.

[1] commit 3e5c291c7942 ("can: add CAN_ERR_CNT flag to notify
    availability of error counter")
Link: https://git.kernel.org/torvalds/linux/c/3e5c291c7942

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 lib.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib.c b/lib.c
index 3c1a0d9..fb08c0b 100644
--- a/lib.c
+++ b/lib.c
@@ -679,7 +679,11 @@ void snprintf_can_error_frame(char *buf, size_t len, const struct canfd_frame *c
 		}
 	}
 
+#ifdef CAN_ERR_CNT
+	if (cf->can_id & CAN_ERR_CNT) {
+#else
 	if (cf->data[6] || cf->data[7]) {
+#endif
 		n += snprintf(buf + n, len - n, "%s", sep);
 		n += snprintf(buf + n, len - n, "error-counter-tx-rx{{%d}{%d}}",
 			      cf->data[6], cf->data[7]);
-- 
2.37.4

[PATCH can-utils-dev v2 4/5] candump: add global variable progname

[Vincent Mailhol]

Instead of using argv[0] several times, make the progname a global
variable.

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 candump.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/candump.c b/candump.c
index 4a239e7..4abd2f6 100644
--- a/candump.c
+++ b/candump.c
@@ -108,6 +108,7 @@ struct if_info { /* bundled information per open socket */
 };
 static struct if_info sock_info[MAXSOCK];
 
+static char *progname;
 static char devname[MAXIFNAMES][IFNAMSIZ+1];
 static int dindex[MAXIFNAMES];
 static int max_devname_len; /* to prevent frazzled device name output */
@@ -121,11 +122,11 @@ extern int optind, opterr, optopt;
 
 static volatile int running = 1;
 
-static void print_usage(char *prg)
+static void print_usage(void)
 {
-	fprintf(stderr, "%s - dump CAN bus traffic.\n", prg);
-	fprintf(stderr, "\nUsage: %s [options] <CAN interface>+\n", prg);
-	fprintf(stderr, "  (use CTRL-C to terminate %s)\n\n", prg);
+	fprintf(stderr, "%s - dump CAN bus traffic.\n", progname);
+	fprintf(stderr, "\nUsage: %s [options] <CAN interface>+\n", progname);
+	fprintf(stderr, "  (use CTRL-C to terminate %s)\n\n", progname);
 	fprintf(stderr, "Options:\n");
 	fprintf(stderr, "         -t <type>   (timestamp: (a)bsolute/(d)elta/(z)ero/(A)bsolute w date)\n");
 	fprintf(stderr, "         -H          (read hardware timestamps instead of system timestamps)\n");
@@ -159,12 +160,12 @@ static void print_usage(char *prg)
 	fprintf(stderr, "Without any given filter all data frames are received ('0:0' default filter).\n");
 	fprintf(stderr, "\nUse interface name '%s' to receive from all CAN interfaces.\n", ANYDEV);
 	fprintf(stderr, "\nExamples:\n");
-	fprintf(stderr, "%s -c -c -ta can0,123:7FF,400:700,#000000FF can2,400~7F0 can3 can8\n\n", prg);
-	fprintf(stderr, "%s -l any,0~0,#FFFFFFFF\n         (log only error frames but no(!) data frames)\n", prg);
-	fprintf(stderr, "%s -l any,0:0,#FFFFFFFF\n         (log error frames and also all data frames)\n", prg);
-	fprintf(stderr, "%s vcan2,12345678:DFFFFFFF\n         (match only for extended CAN ID 12345678)\n", prg);
-	fprintf(stderr, "%s vcan2,123:7FF\n         (matches CAN ID 123 - including EFF and RTR frames)\n", prg);
-	fprintf(stderr, "%s vcan2,123:C00007FF\n         (matches CAN ID 123 - only SFF and non-RTR frames)\n", prg);
+	fprintf(stderr, "%s -c -c -ta can0,123:7FF,400:700,#000000FF can2,400~7F0 can3 can8\n\n", progname);
+	fprintf(stderr, "%s -l any,0~0,#FFFFFFFF\n         (log only error frames but no(!) data frames)\n", progname);
+	fprintf(stderr, "%s -l any,0:0,#FFFFFFFF\n         (log error frames and also all data frames)\n", progname);
+	fprintf(stderr, "%s vcan2,12345678:DFFFFFFF\n         (match only for extended CAN ID 12345678)\n", progname);
+	fprintf(stderr, "%s vcan2,123:7FF\n         (matches CAN ID 123 - including EFF and RTR frames)\n", progname);
+	fprintf(stderr, "%s vcan2,123:C00007FF\n         (matches CAN ID 123 - only SFF and non-RTR frames)\n", progname);
 	fprintf(stderr, "\n");
 }
 
@@ -322,6 +323,8 @@ int main(int argc, char **argv)
 	last_tv.tv_sec = 0;
 	last_tv.tv_usec = 0;
 
+	progname = basename(argv[0]);
+
 	while ((opt = getopt(argc, argv, "t:HciaSs:lDdxLf:n:r:he8T:?")) != -1) {
 		switch (opt) {
 		case 't':
@@ -330,7 +333,7 @@ int main(int argc, char **argv)
 			if ((timestamp != 'a') && (timestamp != 'A') &&
 			    (timestamp != 'd') && (timestamp != 'z')) {
 				fprintf(stderr, "%s: unknown timestamp mode '%c' - ignored\n",
-					basename(argv[0]), optarg[0]);
+					progname, optarg[0]);
 				timestamp = 0;
 			}
 			if ((logtimestamp != 'a') && (logtimestamp != 'z')) {
@@ -369,7 +372,7 @@ int main(int argc, char **argv)
 		case 's':
 			silent = atoi(optarg);
 			if (silent > SILENT_ON) {
-				print_usage(basename(argv[0]));
+				print_usage();
 				exit(1);
 			}
 			break;
@@ -402,7 +405,7 @@ int main(int argc, char **argv)
 		case 'n':
 			count = atoi(optarg);
 			if (count < 1) {
-				print_usage(basename(argv[0]));
+				print_usage();
 				exit(1);
 			}
 			break;
@@ -410,7 +413,7 @@ int main(int argc, char **argv)
 		case 'r':
 			rcvbuf_size = atoi(optarg);
 			if (rcvbuf_size < 1) {
-				print_usage(basename(argv[0]));
+				print_usage();
 				exit(1);
 			}
 			break;
@@ -419,19 +422,19 @@ int main(int argc, char **argv)
 			errno = 0;
 			timeout_ms = strtol(optarg, NULL, 0);
 			if (errno != 0) {
-				print_usage(basename(argv[0]));
+				print_usage();
 				exit(1);
 			}
 			break;
 		default:
-			print_usage(basename(argv[0]));
+			print_usage();
 			exit(1);
 			break;
 		}
 	}
 
 	if (optind == argc) {
-		print_usage(basename(argv[0]));
+		print_usage();
 		exit(0);
 	}
 
-- 
2.37.4

[PATCH can-utils-dev v2 5/5] candump: use linux/net_tstamp.h instead of redefining values ourselves

[Vincent Mailhol]

No need to redefine values locally when available in header.

linux/net_tstamp.h is available since Linux 2.6.30 while socket CAN
was introduced in v2.6.25. These being old releases not maintained any
more for many years, dropping support is acceptable.

Regardless, candump already relies on some other macros defined in
more recent kernel version (e.g. CAN_RAW_ERR_FILTER) meaning that it
would not build on old linux kernel environments.

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 candump.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/candump.c b/candump.c
index 4abd2f6..8f0ca46 100644
--- a/candump.c
+++ b/candump.c
@@ -63,6 +63,7 @@
 
 #include <linux/can.h>
 #include <linux/can/raw.h>
+#include <linux/net_tstamp.h>
 
 #include "terminal.h"
 #include "lib.h"
@@ -72,10 +73,6 @@
 #define SO_TIMESTAMPING 37
 #endif
 
-/* from #include <linux/net_tstamp.h> - since Linux 2.6.30 */
-#define SOF_TIMESTAMPING_SOFTWARE (1 << 4)
-#define SOF_TIMESTAMPING_RX_SOFTWARE (1 << 3)
-#define SOF_TIMESTAMPING_RAW_HARDWARE (1 << 6)
 #define TIMESTAMPSZ 50 /* string 'absolute with date' requires max 49 bytes */
 
 #define MAXSOCK 16    /* max. number of CAN interfaces given on the cmdline */
-- 
2.37.4

[PATCH can-utils-dev v3 0/6] can-utils-dev: a set of update and cleanup patches

[Vincent Mailhol]

This series contain cleanup patches and update of header files. There
is no real relation between each patch (aside of 2/6 and 3/6 which
form a pair) so I will end the cover letter here and let you refer to
the short description.

* Changelog *

v2 -> v3:

  * reorder the patches so that related changes are next to each
    other (especially patch 2/6 and 3/6).

  * patch 2/6: new patch to update include/linux/can/error.h with the
    latest hearder from Linux 6.0.0.

  * patch 3/6: do not use an #ifdef anymore and make sure that the
    code still work if run on old kernel version lower than 6.0.0.

  * patch 6/6: rewrite commit description to point out that there is a
    local copy of the linux headers under include/linux/.

v1 -> v2:

  * patch 2/5: add a note in commit message to explain that slcanpty.c
    debug is deactivated by default.

  * patch 2/5: make the compiler check the pr_debug() syntax even if
    debug is off.

  * reorder the patches so that the lib.c and the candump.c patches
    are next to each other in the series.

Vincent Mailhol (6):
  slcanpty: remove redundant asc2nibble()
  include: update linux/can/error.h
  lib: snprintf_can_error_frame: print counter errors if CAN_ERR_CNT is
    set
  lib: add pr_debug() macro
  candump: add global variable progname
  candump: use linux/net_tstamp.h instead of redefining values ourselves

 Makefile                           |  2 ++
 canbusload.c                       |  9 ++---
 candump.c                          | 58 +++++++++++++-----------------
 canlogserver.c                     | 12 +++----
 include/linux/can/error.h          | 20 ++++++++++-
 lib.c                              |  2 +-
 lib.h                              |  7 ++++
 mcp251xfd/mcp251xfd-dev-coredump.c | 10 +-----
 slcanpty.c                         | 27 +++-----------
 9 files changed, 67 insertions(+), 80 deletions(-)

-- 
2.37.4

[PATCH can-utils-dev v3 1/6] slcanpty: remove redundant asc2nibble()

[Vincent Mailhol]

asc2nibble() is already defined in lib.h. Include lib.h in slcnpty.c
so that asc2nibble() does not need to be redefined a second time and
adjust the Makefile accordingly.

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 Makefile   |  2 ++
 slcanpty.c | 17 ++---------------
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index fb52643..cb67b66 100644
--- a/Makefile
+++ b/Makefile
@@ -119,6 +119,7 @@ canplayer.o:	lib.h
 cansend.o:	lib.h
 log2asc.o:	lib.h
 log2long.o:	lib.h
+slcanpty.o:	lib.h
 j1939acd.o:	libj1939.h
 j1939cat.o:	libj1939.h
 j1939spy.o:	libj1939.h
@@ -135,6 +136,7 @@ cansend:	cansend.o	lib.o
 cansequence:	cansequence.o	lib.o
 log2asc:	log2asc.o	lib.o
 log2long:	log2long.o	lib.o
+slcanpty:	slcanpty.o	lib.o
 j1939acd:	j1939acd.o	libj1939.o
 j1939cat:	j1939cat.o	libj1939.o
 j1939spy:	j1939spy.o	libj1939.o
diff --git a/slcanpty.c b/slcanpty.c
index e6f1efe..4ac9e8a 100644
--- a/slcanpty.c
+++ b/slcanpty.c
@@ -43,27 +43,14 @@
 #include <linux/can/raw.h>
 #include <linux/sockios.h>
 
+#include "lib.h"
+
 /* maximum rx buffer len: extended CAN frame with timestamp */
 #define SLC_MTU (sizeof("T1111222281122334455667788EA5F\r")+1)
 #define DEVICE_NAME_PTMX "/dev/ptmx"
 
 #define DEBUG
 
-static int asc2nibble(char c)
-{
-
-	if ((c >= '0') && (c <= '9'))
-		return c - '0';
-
-	if ((c >= 'A') && (c <= 'F'))
-		return c - 'A' + 10;
-
-	if ((c >= 'a') && (c <= 'f'))
-		return c - 'a' + 10;
-
-	return 16; /* error */
-}
-
 /* read data from pty, send CAN frames to CAN socket and answer commands */
 int pty2can(int pty, int socket, struct can_filter *fi,
 	    int *is_open, int *tstamp)
-- 
2.37.4

[PATCH can-utils-dev v3 2/6] include: update linux/can/error.h

[Vincent Mailhol]

Update the linux/can/error.h hearder to incorporate the changes made
in Linux 6.0.0. Namely:

  - commit 3e5c291c7942 ("can: add CAN_ERR_CNT flag to notify
    availability of error counter")
    Link: https://git.kernel.org/torvalds/linux/c/3e5c291c7942

  - commit 3f9c26210cf8 ("can: error: add definitions for the
    different CAN error thresholds")
    Link: https://git.kernel.org/torvalds/linux/c/3f9c26210cf8

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 include/linux/can/error.h | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/include/linux/can/error.h b/include/linux/can/error.h
index 3463328..acc1ac3 100644
--- a/include/linux/can/error.h
+++ b/include/linux/can/error.h
@@ -57,6 +57,8 @@
 #define CAN_ERR_BUSOFF       0x00000040U /* bus off */
 #define CAN_ERR_BUSERROR     0x00000080U /* bus error (may flood!) */
 #define CAN_ERR_RESTARTED    0x00000100U /* controller restarted */
+#define CAN_ERR_CNT          0x00000200U /* TX error counter / data[6] */
+					 /* RX error counter / data[7] */
 
 /* arbitration lost in bit ... / data[0] */
 #define CAN_ERR_LOSTARB_UNSPEC   0x00 /* unspecified */
@@ -120,6 +122,22 @@
 #define CAN_ERR_TRX_CANL_SHORT_TO_GND  0x70 /* 0111 0000 */
 #define CAN_ERR_TRX_CANL_SHORT_TO_CANH 0x80 /* 1000 0000 */
 
-/* controller specific additional information / data[5..7] */
+/* data[5] is reserved (do not use) */
+
+/* TX error counter / data[6] */
+/* RX error counter / data[7] */
+
+/* CAN state thresholds
+ *
+ * Error counter	Error state
+ * -----------------------------------
+ * 0 -  95		Error-active
+ * 96 - 127		Error-warning
+ * 128 - 255		Error-passive
+ * 256 and greater	Bus-off
+ */
+#define CAN_ERROR_WARNING_THRESHOLD 96
+#define CAN_ERROR_PASSIVE_THRESHOLD 128
+#define CAN_BUS_OFF_THRESHOLD 256
 
 #endif /* _UAPI_CAN_ERROR_H */
-- 
2.37.4

[PATCH can-utils-dev v3 3/6] lib: snprintf_can_error_frame: print counter errors if CAN_ERR_CNT is set

[Vincent Mailhol]

Since version 6.0.0 (specifically [1]), the kernel has a new flag:
CAN_ERR_CNT to notify whether or not the error counter is set. Use
this to decide whether on not the error-counter-tx-rx should be
printed.

This way, when a driver set data[6] and data[7] to zero, it is
possible to differentiate whether the error counter is not available
or if the drivers simply reported both counters to be zero.

For interoperability reasons, continue to check data[6] and data[7]
against zero so that the code still works with the old behavior if
run on a kernel version lower than 6.0.0.

[1] commit 3e5c291c7942 ("can: add CAN_ERR_CNT flag to notify
    availability of error counter")
Link: https://git.kernel.org/torvalds/linux/c/3e5c291c7942

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 lib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib.c b/lib.c
index 3c1a0d9..0f9b510 100644
--- a/lib.c
+++ b/lib.c
@@ -679,7 +679,7 @@ void snprintf_can_error_frame(char *buf, size_t len, const struct canfd_frame *c
 		}
 	}
 
-	if (cf->data[6] || cf->data[7]) {
+	if (cf->can_id & CAN_ERR_CNT || cf->data[6] || cf->data[7]) {
 		n += snprintf(buf + n, len - n, "%s", sep);
 		n += snprintf(buf + n, len - n, "error-counter-tx-rx{{%d}{%d}}",
 			      cf->data[6], cf->data[7]);
-- 
2.37.4

[PATCH can-utils-dev v3 4/6] lib: add pr_debug() macro

[Vincent Mailhol]

Add the pr_debug() macro so that:

  #ifdef DEBUG
  	printf("foo");
  #endif

can be replaced by:

  	pr_debug("foo");

Apply the pr_debug() macro wherever relevant.

Currently, there is no consensus whether debug messages should be
printed on stdout or stderr. Most of the modules: canbusload.c,
candump.c and canlogserver.c use stdout but
mcp251xfd/mcp251xfd-dev-coredump.c uses stderr. Harmonize the behavior
by following the major trend and make
mcp251xfd/mcp251xfd-dev-coredump.c also output to stdout.

slcanpty.c does a #define DEBUG, meaning that debug is always turned
on for this file. Remove this and make debug an option like every
other files.

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 canbusload.c                       |  9 +++------
 candump.c                          | 16 ++++------------
 canlogserver.c                     | 12 ++++--------
 lib.h                              |  7 +++++++
 mcp251xfd/mcp251xfd-dev-coredump.c | 10 +---------
 slcanpty.c                         | 10 +++-------
 6 files changed, 22 insertions(+), 42 deletions(-)

diff --git a/canbusload.c b/canbusload.c
index e4dfc02..47b62fd 100644
--- a/canbusload.c
+++ b/canbusload.c
@@ -61,6 +61,7 @@
 #include <linux/can.h>
 #include <linux/can/raw.h>
 
+#include "lib.h"
 #include "terminal.h"
 #include "canframelen.h"
 
@@ -310,9 +311,7 @@ int main(int argc, char **argv)
 			return 1;
 		}
 
-#ifdef DEBUG
-		printf("open %d '%s'.\n", i, ptr);
-#endif
+		pr_debug("open %d '%s'.\n", i, ptr);
 
 		s[i] = socket(PF_CAN, SOCK_RAW, CAN_RAW);
 		if (s[i] < 0) {
@@ -358,10 +357,8 @@ int main(int argc, char **argv)
 		if (nbytes > max_bitrate_len)
 			max_bitrate_len = nbytes; /* for nice printing */
 
+		pr_debug("using interface name '%s'.\n", ifr.ifr_name);
 
-#ifdef DEBUG
-		printf("using interface name '%s'.\n", ifr.ifr_name);
-#endif
 		/* try to switch the socket into CAN FD mode */
 		const int canfd_on = 1;
 		setsockopt(s[i], SOL_CAN_RAW, CAN_RAW_FD_FRAMES, &canfd_on, sizeof(canfd_on));
diff --git a/candump.c b/candump.c
index 6fe30bb..4a239e7 100644
--- a/candump.c
+++ b/candump.c
@@ -216,9 +216,7 @@ static int idx2dindex(int ifidx, int socket)
 
 	strcpy(devname[i], ifr.ifr_name);
 
-#ifdef DEBUG
-	printf("new index %d (%s)\n", i, devname[i]);
-#endif
+	pr_debug("new index %d (%s)\n", i, devname[i]);
 
 	return i;
 }
@@ -474,9 +472,7 @@ int main(int argc, char **argv)
 		ptr = argv[optind+i];
 		nptr = strchr(ptr, ',');
 
-#ifdef DEBUG
-		printf("open %d '%s'.\n", i, ptr);
-#endif
+		pr_debug("open %d '%s'.\n", i, ptr);
 
 		obj->s = socket(PF_CAN, SOCK_RAW, CAN_RAW);
 		if (obj->s < 0) {
@@ -510,9 +506,7 @@ int main(int argc, char **argv)
 		memset(&ifr.ifr_name, 0, sizeof(ifr.ifr_name));
 		strncpy(ifr.ifr_name, ptr, nbytes);
 
-#ifdef DEBUG
-		printf("using interface name '%s'.\n", ifr.ifr_name);
-#endif
+		pr_debug("using interface name '%s'.\n", ifr.ifr_name);
 
 		if (strcmp(ANYDEV, ifr.ifr_name) != 0) {
 			if (ioctl(obj->s, SIOCGIFINDEX, &ifr) < 0) {
@@ -602,9 +596,7 @@ int main(int argc, char **argv)
 			/* try SO_RCVBUFFORCE first, if we run with CAP_NET_ADMIN */
 			if (setsockopt(obj->s, SOL_SOCKET, SO_RCVBUFFORCE,
 				       &rcvbuf_size, sizeof(rcvbuf_size)) < 0) {
-#ifdef DEBUG
-				printf("SO_RCVBUFFORCE failed so try SO_RCVBUF ...\n");
-#endif
+				pr_debug("SO_RCVBUFFORCE failed so try SO_RCVBUF ...\n");
 				if (setsockopt(obj->s, SOL_SOCKET, SO_RCVBUF,
 					       &rcvbuf_size, sizeof(rcvbuf_size)) < 0) {
 					perror("setsockopt SO_RCVBUF");
diff --git a/canlogserver.c b/canlogserver.c
index 6425ca4..51d548f 100644
--- a/canlogserver.c
+++ b/canlogserver.c
@@ -145,9 +145,7 @@ int idx2dindex(int ifidx, int socket)
 
 	strcpy(devname[i], ifr.ifr_name);
 
-#ifdef DEBUG
-	printf("new index %d (%s)\n", i, devname[i]);
-#endif
+	pr_debug("new index %d (%s)\n", i, devname[i]);
 
 	return i;
 }
@@ -310,11 +308,9 @@ int main(int argc, char **argv)
 
 	for (i=0; i<currmax; i++) {
 
-#ifdef DEBUG
-		printf("open %d '%s' m%08X v%08X i%d e%d.\n",
-		       i, argv[optind+i], mask[i], value[i],
-		       inv_filter[i], err_mask[i]);
-#endif
+		pr_debug("open %d '%s' m%08X v%08X i%d e%d.\n",
+		      i, argv[optind+i], mask[i], value[i],
+		      inv_filter[i], err_mask[i]);
 
 		if ((s[i] = socket(PF_CAN, SOCK_RAW, CAN_RAW)) < 0) {
 			perror("socket");
diff --git a/lib.h b/lib.h
index a4d3ce5..6cc58a2 100644
--- a/lib.h
+++ b/lib.h
@@ -47,6 +47,13 @@
 
 #include <stdio.h>
 
+#ifdef DEBUG
+#define pr_debug(fmt, args...) printf(fmt, ##args)
+#else
+__attribute__((format (printf, 1, 2)))
+static inline int pr_debug(const char* fmt, ...) {return 0;}
+#endif
+
 /* buffer sizes for CAN frame string representations */
 
 #define CL_ID (sizeof("12345678##1"))
diff --git a/mcp251xfd/mcp251xfd-dev-coredump.c b/mcp251xfd/mcp251xfd-dev-coredump.c
index 5874d24..422900f 100644
--- a/mcp251xfd/mcp251xfd-dev-coredump.c
+++ b/mcp251xfd/mcp251xfd-dev-coredump.c
@@ -17,18 +17,10 @@
 
 #include <linux/kernel.h>
 
+#include "../lib.h"
 #include "mcp251xfd.h"
 #include "mcp251xfd-dump-userspace.h"
 
-#define pr_err(fmt, args...)    fprintf(stderr, fmt, ##args)
-#define pr_no(fmt, args...)     while (0) { fprintf(stdout, fmt, ##args); }
-
-#ifdef DEBUG
-#define pr_debug(fmt, args...) pr_err(fmt, ##args)
-#else
-#define pr_debug(fmt, args...) pr_no(fmt, ##args)
-#endif
-
 
 struct mcp251xfd_dump_iter {
 	const void *start;
diff --git a/slcanpty.c b/slcanpty.c
index 4ac9e8a..fa97cd6 100644
--- a/slcanpty.c
+++ b/slcanpty.c
@@ -49,8 +49,6 @@
 #define SLC_MTU (sizeof("T1111222281122334455667788EA5F\r")+1)
 #define DEVICE_NAME_PTMX "/dev/ptmx"
 
-#define DEBUG
-
 /* read data from pty, send CAN frames to CAN socket and answer commands */
 int pty2can(int pty, int socket, struct can_filter *fi,
 	    int *is_open, int *tstamp)
@@ -106,14 +104,12 @@ rx_restart:
 	cmd = buf[0];
 	buf[nbytes] = 0;
 
-#ifdef DEBUG
 	for (tmp = 0; tmp < nbytes; tmp++)
 		if (buf[tmp] == '\r')
-			putchar('@');
+			pr_debug("@");
 		else
-			putchar(buf[tmp]);
-	printf("\n");
-#endif
+			pr_debug("%c", buf[tmp]);
+	pr_debug("\n");
 
 	/* check for filter configuration commands */
 	if (cmd == 'm' || cmd == 'M') {
-- 
2.37.4

[PATCH can-utils-dev v3 5/6] candump: add global variable progname

[Vincent Mailhol]

Instead of using argv[0] several time, make the progname a global
variable.

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 candump.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/candump.c b/candump.c
index 4a239e7..4abd2f6 100644
--- a/candump.c
+++ b/candump.c
@@ -108,6 +108,7 @@ struct if_info { /* bundled information per open socket */
 };
 static struct if_info sock_info[MAXSOCK];
 
+static char *progname;
 static char devname[MAXIFNAMES][IFNAMSIZ+1];
 static int dindex[MAXIFNAMES];
 static int max_devname_len; /* to prevent frazzled device name output */
@@ -121,11 +122,11 @@ extern int optind, opterr, optopt;
 
 static volatile int running = 1;
 
-static void print_usage(char *prg)
+static void print_usage(void)
 {
-	fprintf(stderr, "%s - dump CAN bus traffic.\n", prg);
-	fprintf(stderr, "\nUsage: %s [options] <CAN interface>+\n", prg);
-	fprintf(stderr, "  (use CTRL-C to terminate %s)\n\n", prg);
+	fprintf(stderr, "%s - dump CAN bus traffic.\n", progname);
+	fprintf(stderr, "\nUsage: %s [options] <CAN interface>+\n", progname);
+	fprintf(stderr, "  (use CTRL-C to terminate %s)\n\n", progname);
 	fprintf(stderr, "Options:\n");
 	fprintf(stderr, "         -t <type>   (timestamp: (a)bsolute/(d)elta/(z)ero/(A)bsolute w date)\n");
 	fprintf(stderr, "         -H          (read hardware timestamps instead of system timestamps)\n");
@@ -159,12 +160,12 @@ static void print_usage(char *prg)
 	fprintf(stderr, "Without any given filter all data frames are received ('0:0' default filter).\n");
 	fprintf(stderr, "\nUse interface name '%s' to receive from all CAN interfaces.\n", ANYDEV);
 	fprintf(stderr, "\nExamples:\n");
-	fprintf(stderr, "%s -c -c -ta can0,123:7FF,400:700,#000000FF can2,400~7F0 can3 can8\n\n", prg);
-	fprintf(stderr, "%s -l any,0~0,#FFFFFFFF\n         (log only error frames but no(!) data frames)\n", prg);
-	fprintf(stderr, "%s -l any,0:0,#FFFFFFFF\n         (log error frames and also all data frames)\n", prg);
-	fprintf(stderr, "%s vcan2,12345678:DFFFFFFF\n         (match only for extended CAN ID 12345678)\n", prg);
-	fprintf(stderr, "%s vcan2,123:7FF\n         (matches CAN ID 123 - including EFF and RTR frames)\n", prg);
-	fprintf(stderr, "%s vcan2,123:C00007FF\n         (matches CAN ID 123 - only SFF and non-RTR frames)\n", prg);
+	fprintf(stderr, "%s -c -c -ta can0,123:7FF,400:700,#000000FF can2,400~7F0 can3 can8\n\n", progname);
+	fprintf(stderr, "%s -l any,0~0,#FFFFFFFF\n         (log only error frames but no(!) data frames)\n", progname);
+	fprintf(stderr, "%s -l any,0:0,#FFFFFFFF\n         (log error frames and also all data frames)\n", progname);
+	fprintf(stderr, "%s vcan2,12345678:DFFFFFFF\n         (match only for extended CAN ID 12345678)\n", progname);
+	fprintf(stderr, "%s vcan2,123:7FF\n         (matches CAN ID 123 - including EFF and RTR frames)\n", progname);
+	fprintf(stderr, "%s vcan2,123:C00007FF\n         (matches CAN ID 123 - only SFF and non-RTR frames)\n", progname);
 	fprintf(stderr, "\n");
 }
 
@@ -322,6 +323,8 @@ int main(int argc, char **argv)
 	last_tv.tv_sec = 0;
 	last_tv.tv_usec = 0;
 
+	progname = basename(argv[0]);
+
 	while ((opt = getopt(argc, argv, "t:HciaSs:lDdxLf:n:r:he8T:?")) != -1) {
 		switch (opt) {
 		case 't':
@@ -330,7 +333,7 @@ int main(int argc, char **argv)
 			if ((timestamp != 'a') && (timestamp != 'A') &&
 			    (timestamp != 'd') && (timestamp != 'z')) {
 				fprintf(stderr, "%s: unknown timestamp mode '%c' - ignored\n",
-					basename(argv[0]), optarg[0]);
+					progname, optarg[0]);
 				timestamp = 0;
 			}
 			if ((logtimestamp != 'a') && (logtimestamp != 'z')) {
@@ -369,7 +372,7 @@ int main(int argc, char **argv)
 		case 's':
 			silent = atoi(optarg);
 			if (silent > SILENT_ON) {
-				print_usage(basename(argv[0]));
+				print_usage();
 				exit(1);
 			}
 			break;
@@ -402,7 +405,7 @@ int main(int argc, char **argv)
 		case 'n':
 			count = atoi(optarg);
 			if (count < 1) {
-				print_usage(basename(argv[0]));
+				print_usage();
 				exit(1);
 			}
 			break;
@@ -410,7 +413,7 @@ int main(int argc, char **argv)
 		case 'r':
 			rcvbuf_size = atoi(optarg);
 			if (rcvbuf_size < 1) {
-				print_usage(basename(argv[0]));
+				print_usage();
 				exit(1);
 			}
 			break;
@@ -419,19 +422,19 @@ int main(int argc, char **argv)
 			errno = 0;
 			timeout_ms = strtol(optarg, NULL, 0);
 			if (errno != 0) {
-				print_usage(basename(argv[0]));
+				print_usage();
 				exit(1);
 			}
 			break;
 		default:
-			print_usage(basename(argv[0]));
+			print_usage();
 			exit(1);
 			break;
 		}
 	}
 
 	if (optind == argc) {
-		print_usage(basename(argv[0]));
+		print_usage();
 		exit(0);
 	}
 
-- 
2.37.4

[PATCH can-utils-dev v3 6/6] candump: use linux/net_tstamp.h instead of redefining values ourselves

[Vincent Mailhol]

The project contains a local copy of the timestamp header at
include/linux/net_tstamp.h. However, candump.c redefines the
net_tstamp.h values instead of relying on the header.

Replace these by a "include <linux/net_tstamp.h>".

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 candump.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/candump.c b/candump.c
index 4abd2f6..8f0ca46 100644
--- a/candump.c
+++ b/candump.c
@@ -63,6 +63,7 @@
 
 #include <linux/can.h>
 #include <linux/can/raw.h>
+#include <linux/net_tstamp.h>
 
 #include "terminal.h"
 #include "lib.h"
@@ -72,10 +73,6 @@
 #define SO_TIMESTAMPING 37
 #endif
 
-/* from #include <linux/net_tstamp.h> - since Linux 2.6.30 */
-#define SOF_TIMESTAMPING_SOFTWARE (1 << 4)
-#define SOF_TIMESTAMPING_RX_SOFTWARE (1 << 3)
-#define SOF_TIMESTAMPING_RAW_HARDWARE (1 << 6)
 #define TIMESTAMPSZ 50 /* string 'absolute with date' requires max 49 bytes */
 
 #define MAXSOCK 16    /* max. number of CAN interfaces given on the cmdline */
-- 
2.37.4

Re: [PATCH can-utils-dev v3 0/6] can-utils-dev: a set of update and cleanup patches

[Oliver Hartkopp]

On 14.11.22 17:38, Vincent Mailhol wrote:
> This series contain cleanup patches and update of header files. There
> is no real relation between each patch (aside of 2/6 and 3/6 which
> form a pair) so I will end the cover letter here and let you refer to
> the short description.

Nice cleanups.

Thanks Vincent!

Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>

> 
> * Changelog *
> 
> v2 -> v3:
> 
>    * reorder the patches so that related changes are next to each
>      other (especially patch 2/6 and 3/6).
> 
>    * patch 2/6: new patch to update include/linux/can/error.h with the
>      latest hearder from Linux 6.0.0.
> 
>    * patch 3/6: do not use an #ifdef anymore and make sure that the
>      code still work if run on old kernel version lower than 6.0.0.
> 
>    * patch 6/6: rewrite commit description to point out that there is a
>      local copy of the linux headers under include/linux/.
> 
> v1 -> v2:
> 
>    * patch 2/5: add a note in commit message to explain that slcanpty.c
>      debug is deactivated by default.
> 
>    * patch 2/5: make the compiler check the pr_debug() syntax even if
>      debug is off.
> 
>    * reorder the patches so that the lib.c and the candump.c patches
>      are next to each other in the series.
> 
> Vincent Mailhol (6):
>    slcanpty: remove redundant asc2nibble()
>    include: update linux/can/error.h
>    lib: snprintf_can_error_frame: print counter errors if CAN_ERR_CNT is
>      set
>    lib: add pr_debug() macro
>    candump: add global variable progname
>    candump: use linux/net_tstamp.h instead of redefining values ourselves
> 
>   Makefile                           |  2 ++
>   canbusload.c                       |  9 ++---
>   candump.c                          | 58 +++++++++++++-----------------
>   canlogserver.c                     | 12 +++----
>   include/linux/can/error.h          | 20 ++++++++++-
>   lib.c                              |  2 +-
>   lib.h                              |  7 ++++
>   mcp251xfd/mcp251xfd-dev-coredump.c | 10 +-----
>   slcanpty.c                         | 27 +++-----------
>   9 files changed, 67 insertions(+), 80 deletions(-)
> 

Re: [PATCH can-utils-dev v3 0/6] can-utils-dev: a set of update and cleanup patches

[Marc Kleine-Budde]

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

On 15.11.2022 01:38:42, Vincent Mailhol wrote:
> This series contain cleanup patches and update of header files. There
> is no real relation between each patch (aside of 2/6 and 3/6 which
> form a pair) so I will end the cover letter here and let you refer to
> the short description.

Applied to master.

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

Re: [PATCH can-utils-dev v3 0/6] can-utils-dev: a set of update and cleanup patches

[Marc Kleine-Budde]

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

On 15.11.2022 09:15:45, Marc Kleine-Budde wrote:
> On 15.11.2022 01:38:42, Vincent Mailhol wrote:
> > This series contain cleanup patches and update of header files. There
> > is no real relation between each patch (aside of 2/6 and 3/6 which
> > form a pair) so I will end the cover letter here and let you refer to
> > the short description.
> 
> Applied to master.

Squashed the following changes:

> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 7088c5120154..a4e8484db931 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -33,6 +33,7 @@ set(PROGRAMS_CANLIB
>      cansequence
>      log2asc
>      log2long
> +    slcanpty
>  )
>  
>  set(PROGRAMS_J1939
> @@ -58,7 +59,6 @@ set(PROGRAMS
>      isotptun
>      slcan_attach
>      slcand
> -    slcanpty
>  )
>  
>  add_executable(can-calc-bit-timing

Fixed linking with cmake

> diff --git a/Makefile b/Makefile
> index c3baa6b287fc..529343d576df 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -46,6 +46,7 @@ MAKEFLAGS := -k
>  CFLAGS := -O2 -Wall -Wno-parentheses
>  
>  CPPFLAGS += \
> +       -I. \
>         -Iinclude \
>         -DAF_CAN=PF_CAN \
>         -DPF_CAN=29 \

Add the missing include search path to for mcp251xfd-dev-coredump...:

> diff --git a/mcp251xfd/mcp251xfd-dev-coredump.c b/mcp251xfd/mcp251xfd-dev-coredump.c
> index 422900f9cb9e..680c73479d1f 100644
> --- a/mcp251xfd/mcp251xfd-dev-coredump.c
> +++ b/mcp251xfd/mcp251xfd-dev-coredump.c
> @@ -17,7 +17,7 @@
>  
>  #include <linux/kernel.h>
>  
> -#include "../lib.h"
> +#include "lib.h"
>  #include "mcp251xfd.h"
>  #include "mcp251xfd-dump-userspace.h"
>

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