On 1/25/26 10:33 AM, Dmitry Mityugov wrote: > Array impl_set_masked_funcs is defined in lib/odp-execute-avx512.c > without static modifier but isn't used outside of this module. Recent > versions of GCC can be instructed to deliver warnings for such cases, > with -Wmissing-variable-declarations compiler option enabled. The > problem in question can be reproduced with > > ./configure --enable-Werror CFLAGS="-Wmissing-variable-declarations" && make > > To fix the problem, the array should be defined as static. This problem > can be reproduced only on 64-bit x86 machines, not on ARM/RISC-V. > > Signed-off-by: Dmitry Mityugov <[email protected]> > --- > Huge thanks to everyone who helped me fix the email with the original > patch. Hopefully it will be accepted this time. > > Not sure about the address this patch should be sent to. Page > https://mail.openvswitch.org/mailman/listinfo/ovs-dev says to use > [email protected] whereas this guide > https://docs.openvswitch.org/en/latest/internals/contributing/submitting-patches/ > recommends [email protected] > > lib/odp-execute-avx512.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c > index 13afe0c79..55af58737 100644 > --- a/lib/odp-execute-avx512.c > +++ b/lib/odp-execute-avx512.c > @@ -108,7 +108,7 @@ BUILD_ASSERT_DECL(offsetof(struct ovs_key_ipv6, > ipv6_tclass) + > offsetof(struct ovs_key_ipv6, ipv6_hlimit)); > > /* Array of callback functions, one for each masked operation. */ > -odp_execute_action_cb impl_set_masked_funcs[__OVS_KEY_ATTR_MAX]; > +static odp_execute_action_cb impl_set_masked_funcs[__OVS_KEY_ATTR_MAX]; > > static inline void ALWAYS_INLINE > avx512_dp_packet_resize_l2(struct dp_packet *b, int resize_by_bytes)
Hi, Dmitry. Thanks for the the patch! I agree that we should fix the missing keyword in this file, but it doesn't make much sense if we have no protection from re-introducing the issue later in some other place. We already enable by default the -Wmissing-prototypes and the -Wmissing-field-initializers, so I think, we should just turn all -Wmissing-variable-declarations whenever available as well. This will help developers and our CI to spot the problem. See the relevant code in configure.ac, look for OVS_ENABLE_OPTION. Note that we'll need to change the test program in _OVS_CHECK_CC_OPTION as well, since it's not compliant with this flag and the detection will fail otherwise. The flag is also supported by clang for a long time, some GCC part in the subject is not particularly relevant. The new patch name prefix should be 'configure:' with the name focusing on enabling the new option. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
