Em Thu, Jan 31, 2019 at 07:12:33PM +0000, Yonghong Song escreveu:

On 1/31/19 10:52 AM, Alexei Starovoitov wrote:

quoted

On Tue, Jan 29, 2019 at 04:12:15PM +0100, Magnus Karlsson wrote:

quoted

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.karlsson@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