On 2/1/19 11:02 AM, Andrii Nakryiko wrote:
> On Fri, Feb 1, 2019 at 10:16 AM Yonghong Song <y...@fb.com> wrote:
>>
>> Currently, the libbpf API function libbpf_set_print()
>> takes three function pointer parameters for warning, info
>> and debug printout respectively.
>>
>> This patch changes the API to have just one function pointer
>> parameter and the function pointer has one additional
>> parameter "debugging level". So if in the future, if
>> the debug level is increased, the function signature
>> won't change.
>>
>> Signed-off-by: Yonghong Song <y...@fb.com>
>> ---
>> tools/lib/bpf/libbpf.c | 28 ++++-----------
>> tools/lib/bpf/libbpf.h | 14 +++-----
>> tools/lib/bpf/test_libbpf.cpp | 2 +-
>> tools/perf/util/bpf-loader.c | 32 +++++++----------
>> tools/testing/selftests/bpf/test_btf.c | 7 ++--
>> .../testing/selftests/bpf/test_libbpf_open.c | 36 +++++++++----------
>> tools/testing/selftests/bpf/test_progs.c | 20 +++++++++--
>> 7 files changed, 63 insertions(+), 76 deletions(-)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 1b1c0b504d25..d2337a179837 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -54,8 +54,8 @@
>>
>> #define __printf(a, b) __attribute__((format(printf, a, b)))
>>
>> -__printf(1, 2)
>> -static int __base_pr(const char *format, ...)
>> +__printf(2, 3)
>> +static int __base_pr(enum libbpf_print_level level, const char *format, ...)
>> {
>> va_list args;
>> int err;
>> @@ -66,17 +66,11 @@ static int __base_pr(const char *format, ...)
>> 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;
>> +static __printf(2, 3) libbpf_print_fn_t __libbpf_pr = __base_pr;
>>
>> -void libbpf_set_print(libbpf_print_fn_t warn,
>> - libbpf_print_fn_t info,
>> - libbpf_print_fn_t debug)
>> +void libbpf_set_print(libbpf_print_fn_t fn)
>> {
>> - __pr_warning = warn;
>> - __pr_info = info;
>> - __pr_debug = debug;
>> + __libbpf_pr = fn;
>> }
>>
>> __printf(2, 3)
>> @@ -85,16 +79,8 @@ void libbpf_debug_print(enum libbpf_print_level level,
>> const char *format, ...)
>> va_list args;
>>
>> va_start(args, format);
>> - if (level == LIBBPF_WARN) {
>> - if (__pr_warning)
>> - __pr_warning(format, args);
>> - } else if (level == LIBBPF_INFO) {
>> - if (__pr_info)
>> - __pr_info(format, args);
>> - } else {
>> - if (__pr_debug)
>> - __pr_debug(format, args);
>> - }
>> + if (__libbpf_pr)
>
> If __libbpf_pr is NULL, is there a need to call va_start/va_end? If
> not, should they be moved inside if's body?
You are right. Will fix this in the next version.
>
>> + __libbpf_pr(level, format, args);
>> va_end(args);
>> }
>>
