> On 1/27/24 10:50 AM, Jose E. Marchesi wrote:
>> The generated bpf_helper_defs.h file currently contains definitions
>> like this for the kernel helpers, which are static objects:
>>
>> static void *(*bpf_map_lookup_elem)(void *map, const void *key) = (void *) 1;
>>
>> These work well in both clang and GCC because both compilers do
>> constant propagation with -O1 and higher optimization, resulting in
>> `call 1' BPF instructions being generated, which are calls to kernel
>> helpers.
>>
>> However, there is a discrepancy on how the -Wunused-variable
>> warning (activated by -Wall) is handled in these compilers:
>>
>> - clang will not emit -Wunused-variable warnings for static variables
>> defined in C header files, be them constant or not constant.
>>
>> - GCC will not emit -Wunused-variable warnings for _constant_ static
>> variables defined in header files, but it will emit warnings for
>> non-constant static variables defined in header files.
>>
>> There is no reason for these bpf_helpers_def.h pointers to not be
>> declared constant, and it is actually desirable to do so, since their
>> values are not to be changed. So this patch modifies bpf_doc.py to
>> generate prototypes like:
>>
>> static void *(* const bpf_map_lookup_elem)(void *map, const void *key) = (void *) 1;
>>
>> This allows GCC to not error while compiling BPF programs with `-Wall
>> -Werror', while still being able to detect and error on legitimate
>> unused variables in the program themselves.
>>
>> This change doesn't impact the desired constant propagation in neither
>> Clang nor GCC with -O1 and higher. On the contrary, being declared as
>> constant may increase the odds they get constant folded when
>> used/referred to in certain circumstances.
>>
>> Tested in bpf-next master.
>> No regressions.
>>
>> Signed-off-by: Jose E. Marchesi <jose.marchesi@xxxxxxxxxx>
>> Cc: alexei.starovoitov@xxxxxxxxx
>> Cc: yonghong.song@xxxxxxxxx
>> Cc: eddyz87@xxxxxxxxx
>> Cc: cupertino.miranda@xxxxxxxxxx
>> Cc: david.faust@xxxxxxxxxx
>
> LGTM. Please add proper tag in the commit subject, e.g.,
> "[PATCH bpf-next]" instead of "[PATCH]", so CI can pick
> up the patch and do proper test.
Sorry about that. I will use the proper Subject in future patches.
>
> Acked-by: Yonghong Song <yonghong.song@xxxxxxxxx>
>
>> ---
>> scripts/bpf_doc.py | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/bpf_doc.py b/scripts/bpf_doc.py
>> index 61b7dddedc46..2b94749c99cc 100755
>> --- a/scripts/bpf_doc.py
>> +++ b/scripts/bpf_doc.py
>> @@ -827,7 +827,7 @@ class PrinterHelpers(Printer):
>> print(' *{}{}'.format(' \t' if line else '', line))
>> print(' */')
>> - print('static %s %s(*%s)(' % (self.map_type(proto['ret_type']),
>> + print('static %s %s(* const %s)(' % (self.map_type(proto['ret_type']),
>> proto['ret_star'], proto['name']), end='')
>> comma = ''
>> for i, a in enumerate(proto['args']):
