netdev - Re: [PATCH bpf-next v3 1/3] tools/bpf: move libbpf pr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190201225227.fxfvmldsxbe632vz@ast-mbp>
Date:   Fri, 1 Feb 2019 14:52:28 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Yonghong Song <yhs@...com>
Cc:     Arnaldo Carvalho de Melo <acme@...hat.com>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        netdev@...r.kernel.org, Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>, kernel-team@...com
Subject: Re: [PATCH bpf-next v3 1/3] tools/bpf: move libbpf pr_* debug print
 functions to headers

On Fri, Feb 01, 2019 at 02:06:16PM -0800, Yonghong Song wrote:
> A global function libbpf_debug_print, which is invisible
> outside the shared library, is defined to print based
> on levels. The pr_warning, pr_info and pr_debug
> macros are moved into the newly created header
> common.h. So any .c file including common.h can
> use these macros directly.

s/common.h/util.h/ ?

I understand that util.h is libbpf's internal header, but still
I'm worried that such file name is too generic and if users
get their header paths wrong they may pick it by accident.
May be call it libbpf_util.h instead?

> 
> Currently btf__new and btf_ext__new API has an argument getting
> __pr_debug function pointer into btf.c so the debugging information
> can be printed there. This patch removed this parameter
> from btf__new and btf_ext__new and directly using pr_debug in btf.c.
> 
> Another global function libbpf_dprint_level_available, also
> invisible outside the shared library, can test
> whether a particular level debug printing is
> available or not. It is used in btf.c to
> test whether DEBUG level debug printing is availabl or not,
> based on which the log buffer will be allocated when loading
> btf to the kernel.
> 
> Signed-off-by: Yonghong Song <yhs@...com>
...
> diff --git a/tools/lib/bpf/util.h b/tools/lib/bpf/util.h
> new file mode 100644
> index 000000000000..3ae80f486875
> --- /dev/null
> +++ b/tools/lib/bpf/util.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
> +/* Copyright (c) 2019 Facebook */
> +
> +#ifndef __LIBBPF_COMMON_H
> +#define __LIBBPF_COMMON_H

s/COMMON/new name/

> +
> +#include <stdbool.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +extern void libbpf_debug_print(enum libbpf_print_level level,
> +			       const char *format, ...)
> +	__attribute__((format(printf, 2, 3)));

May be shorten the name to libbpf_print ?
Such name will match enum libbpf_print_level and libbpf_set_print.

> +
> +extern bool libbpf_dprint_level_available(enum libbpf_print_level level);
> +
> +#define __pr(level, fmt, ...)	\
> +do {				\
> +	libbpf_debug_print(level, "libbpf: " fmt, ##__VA_ARGS__);	\
> +} while (0)
> +
> +#define pr_warning(fmt, ...)	__pr(LIBBPF_WARN, fmt, ##__VA_ARGS__)
> +#define pr_info(fmt, ...)	__pr(LIBBPF_INFO, fmt, ##__VA_ARGS__)
> +#define pr_debug(fmt, ...)	__pr(LIBBPF_DEBUG, fmt, ##__VA_ARGS__)
> +
> +#ifdef __cplusplus
> +} /* extern "C" */
> +#endif
> +
> +#endif
> -- 
> 2.17.1
>