Re: git: e67aef419093 - main - Add DEBUG_PRINTF to stand.h

On Thu, 17 Jul 2025, Simon J. Gerraty wrote:

> The branch main has been updated by sjg:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=e67aef419093b08984b8a2de535bc3e4ce13e087
>
> commit e67aef419093b08984b8a2de535bc3e4ce13e087
> Author:     Simon J. Gerraty <sjg@FreeBSD.org>
> AuthorDate: 2025-07-17 23:36:17 +0000
> Commit:     Simon J. Gerraty <sjg@FreeBSD.org>
> CommitDate: 2025-07-17 23:36:17 +0000
>
>    Add DEBUG_PRINTF to stand.h

Given stand.h is included in other parts of the kernel and I don't know
even how indrectly for the error case I have, this feels problematic.

I know the experimental USB code I am testing has it's own cleamup
to do but I did run into the conflict of DEBUG_PRINTF being re-defined
there.

Given the generic name I wonder if this should be STAND_DPRINTF or
other?   I admit, there's not much code exporting DEBUG_PRINTF or
DPRINTF macros so this may just be the one and only case;  not sure how
much other vendor code is out there...

>    stand/ is mostly debugged with printfs, in an ad hoc and sometimes
>    fragile manner. For example BOOTP_DEBUG in bootp.c cannot be defined
>    unless NETIF_DEBUG is defined in dev_net.c or build fails for lack of the
>    symbol debug.
>
>    The DEBUG_PRINTF implementation in stand.h addresses that and allows
>    for more control over debug output.  It is compatible with the
>    usage in libsecureboot.
>
>    Simply define _DEBUG_LEVEL to the desired level of debug
>    or in the case of libsecureboot _DEBUG_LEVEL_VAR to the variable that
>    will hold that value - default is _debug which is static so each
>    translation unit can be controlled independently.
>
>    The 1st arg to DEBUG_PRINTF is a level which must be greater than or
>    equal to _DEBUG_LEVEL_VAR if the printf is to be called.
>    See libsecureboot for more examples.
>
>    Reviewed by:    imp
>    Sponsored by:   Juniper Networks, Inc.
>    Differential Revision:  https://reviews.freebsd.org/D51269
> ---
> lib/libsecureboot/h/libsecureboot.h |  1 +
> stand/common/dev_net.c              | 57 ++++++++-------------------
> stand/libsa/bootp.c                 | 78 +++++++++----------------------------
> stand/libsa/pkgfs.c                 | 33 +++++++++-------
> stand/libsa/stand.h                 | 13 +++++++
> 5 files changed, 68 insertions(+), 114 deletions(-)
....
> diff --git a/stand/libsa/stand.h b/stand/libsa/stand.h
> index e1188fb73a26..8b7d93074ef2 100644
> --- a/stand/libsa/stand.h
> +++ b/stand/libsa/stand.h
> @@ -558,4 +558,17 @@ void tslog_getbuf(void ** buf, size_t * len);
>
> __END_DECLS
>
> +/* define _DEBUG_LEVEL n or _DEBUG_LEVEL_VAR before include */
> +#ifndef DEBUG_PRINTF
> +# if defined(_DEBUG_LEVEL) || defined(_DEBUG_LEVEL_VAR)
> +#   ifndef _DEBUG_LEVEL_VAR
> +#     define _DEBUG_LEVEL_VAR _debug
> +static int _debug = _DEBUG_LEVEL;
> +#   endif
> +#   define DEBUG_PRINTF(n, args) if (_DEBUG_LEVEL_VAR >= n) printf args
> +# else
> +#   define DEBUG_PRINTF(n, args)
> +# endif
> +#endif
> +
> #endif	/* STAND_H */
>

-- 
Bjoern A. Zeeb                                                     r15:7