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 |