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@xxxxxxxxxxxxxx>
> Signed-off-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>
> ---
> @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:
signature.asc
Description: PGP signature
