On 2/4/19 5:51 PM, Alexei Starovoitov wrote:
> On Tue, Feb 05, 2019 at 12:37:29AM +0000, Yonghong Song wrote:
>>
>>
>> On 2/4/19 4:20 PM, Stanislav Fomichev wrote:
>>> With the recent print rework we now have the following problem:
>>> pr_{warning,info,debug} expand to __pr which calls libbpf_print.
>>> libbpf_print does va_start and calls __libbpf_pr with va_list argument.
>>> In __base_pr we again do va_start. Because the next argument is a
>>> va_list, we don't get correct pointer to the argument (and print noting
>>> in my case, I don't know why it doesn't crash tbh).
>>>
>>> Fix this by changing libbpf_print_fn_t signature to accept va_list and
>>> remove unneeded calls to va_start in the existing users.
>>>
>>> Alternatively, this can we solved by exporting __libbpf_pr and
>>> changing __pr macro to (and killing libbpf_print):
>>> {
>>> if (__libbpf_pr)
>>> __libbpf_pr(level, "libbpf: " fmt, ##__VA_ARGS__)
>>> }
>>>
>>> Signed-off-by: Stanislav Fomichev <s...@google.com>
>>
>> It is my mistake. My early version did passed correctly and later
>> on I made some changes and did not test properly. Thanks for the fix!
>>
>> Acked-by: Yonghong Song <y...@fb.com>
>
> argh.
> Applied. Thanks for the fix.
> Yonghong, how was the earlier patch set tested?
Before the global function patch set, I have a global variable version,
which I tested and worked. Later on when changing to global
libbpf_print approach, I tested it through test_btf. That is why
I found a missing check for LIBBPF_DEBUG level in default __base_pr.
But I have to admit that I probably did not pay attention to contents
somehow so I missed the garbled output.
> It sounds that nothing should have worked.
> How perf changes were tested?
I only tested compilation. The context is similar to a few other
bpf selftests programs and I assume with similar implementation, the
result should be similar. But really badly, they are all incorrect :-(
