Em Thu, Jan 31, 2019 at 07:12:33PM +0000, Yonghong Song escreveu:
>
>
> On 1/31/19 10:52 AM, Alexei Starovoitov wrote:
> > On Tue, Jan 29, 2019 at 04:12:15PM +0100, Magnus Karlsson wrote:
> >> Move the pr_*() functions in libbpf.c to a common header file called
> >> libbpf_internal.h. This so that the later libbpf AF_XDP helper library
> >> code in xsk.c can use these printing functions too.
> >>
> >> Signed-off-by: Magnus Karlsson <magnus.karls...@intel.com>
> >> ---
> >> tools/lib/bpf/libbpf.c | 30 +-----------------------------
> >> tools/lib/bpf/libbpf_internal.h | 41
> >> +++++++++++++++++++++++++++++++++++++++++
> >> 2 files changed, 42 insertions(+), 29 deletions(-)
> >> create mode 100644 tools/lib/bpf/libbpf_internal.h
> >>
> >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> >> index 2ccde17..1d7fe26 100644
> >> --- a/tools/lib/bpf/libbpf.c
> >> +++ b/tools/lib/bpf/libbpf.c
> >> @@ -39,6 +39,7 @@
> >> #include <gelf.h>
> >>
> >> #include "libbpf.h"
> >> +#include "libbpf_internal.h"
> >> #include "bpf.h"
> >> #include "btf.h"
> >> #include "str_error.h"
> >> @@ -51,34 +52,6 @@
> >> #define BPF_FS_MAGIC 0xcafe4a11
> >> #endif
> >>
> >> -#define __printf(a, b) __attribute__((format(printf, a, b)))
> >> -
> >> -__printf(1, 2)
> >> -static int __base_pr(const char *format, ...)
> >> -{
> >> - va_list args;
> >> - int err;
> >> -
> >> - va_start(args, format);
> >> - err = vfprintf(stderr, format, args);
> >> - va_end(args);
> >> - return err;
> >> -}
> >> -
> >> -static __printf(1, 2) libbpf_print_fn_t __pr_warning = __base_pr;
> >> -static __printf(1, 2) libbpf_print_fn_t __pr_info = __base_pr;
> >> -static __printf(1, 2) libbpf_print_fn_t __pr_debug;
> >> -
> >> -#define __pr(func, fmt, ...) \
> >> -do { \
> >> - if ((func)) \
> >> - (func)("libbpf: " fmt, ##__VA_ARGS__); \
> >> -} while (0)
> >> -
> >> -#define pr_warning(fmt, ...) __pr(__pr_warning, fmt, ##__VA_ARGS__)
> >> -#define pr_info(fmt, ...) __pr(__pr_info, fmt, ##__VA_ARGS__)
> >> -#define pr_debug(fmt, ...) __pr(__pr_debug, fmt, ##__VA_ARGS__)
> >
> > since these funcs are about to be used more widely
> > let's clean this api while we still can.
> > How about we convert it to single pr_log callback function
> > with verbosity flag instead of three callbacks ?
Probably. I just wanted to keep, in the source code, the
pr_{debug,warn,info} APIs, to reduce the learning curve for people used
to program in the kernel and in tools/perf/.
> Another possible change related to the API function
> libbpf_set_print
>
> Currently the function takes three parameters,
>
> LIBBPF_API void libbpf_set_print(libbpf_print_fn_t warn,
> libbpf_print_fn_t info,
> libbpf_print_fn_t debug);
>
> So it currently supports three level of debugging output.
> Is it possible in the future more debugging output level
> may be supported? if this is the case, maybe we could
> change the API function libbpf_set_print to something like
> the below before the library version bumps into 1.0.0?
>
> LIBBPF_API void libbpf_set_print(libbpf_print_fn_t dprint);
> and
> typedef int (*libbpf_print_fn_t)(enum libbpf_debug_level level,
> const char *, ...)
> __attribute__((format(printf, 1, 2)));
> enum libbpf_debug_level {
> LIBBPF_WARN,
> LIBBPF_INFO,
> LIBBPF_DEBUG,
> };
>
> Basically, the user provided callback function must have
> the first parameters as the level.
>
> Any comments? Arnaldo?
I think it is ok, as long as we can override the pr_log thing to allow
for using with TUI, stdio, GUI.
- Arnaldo
