| Message ID | 1572992064-2249-1-git-send-email-gvrose8192@gmail.com |
|---|---|
| State | Accepted |
| Commit | 89a84caf9ce9e64cef74e9e71ce3aeacb721795c |
| Headers | show |
| Series | [ovs-dev] compat: Add missing inline keyword | expand |
On Tue, Nov 05, 2019 at 02:14:24PM -0800, Greg Rose wrote: > The missing inline keyword before the definition of the > rpl_nf_ct_tmpl_free() function causes spurious warnings about the > function not being used on some older kernels. Add the keyword > to suppress the warning. > > Signed-off-by: Greg Rose <gvrose8192@gmail.com> The commit message and the patch make sense given my experience of GCC behavior. I haven't tested it. Acked-by: Ben Pfaff <blp@ovn.org>
On 11/5/2019 3:14 PM, Ben Pfaff wrote: > On Tue, Nov 05, 2019 at 02:14:24PM -0800, Greg Rose wrote: >> The missing inline keyword before the definition of the >> rpl_nf_ct_tmpl_free() function causes spurious warnings about the >> function not being used on some older kernels. Add the keyword >> to suppress the warning. >> >> Signed-off-by: Greg Rose <gvrose8192@gmail.com> > The commit message and the patch make sense given my experience of GCC > behavior. I haven't tested it. > > Acked-by: Ben Pfaff <blp@ovn.org> I only did a compile test of it on Travis. https://travis-ci.org/gvrose8192/ovs-experimental/builds/607794710 There's one error but it's a resource or infrastructure issue I think. Thanks, - Greg
On 11/5/2019 5:03 PM, Gregory Rose wrote: > > On 11/5/2019 3:14 PM, Ben Pfaff wrote: >> On Tue, Nov 05, 2019 at 02:14:24PM -0800, Greg Rose wrote: >>> The missing inline keyword before the definition of the >>> rpl_nf_ct_tmpl_free() function causes spurious warnings about the >>> function not being used on some older kernels. Add the keyword >>> to suppress the warning. >>> >>> Signed-off-by: Greg Rose <gvrose8192@gmail.com> >> The commit message and the patch make sense given my experience of GCC >> behavior. I haven't tested it. >> >> Acked-by: Ben Pfaff <blp@ovn.org> > > I only did a compile test of it on Travis. > > https://travis-ci.org/gvrose8192/ovs-experimental/builds/607794710 > > There's one error but it's a resource or infrastructure issue I think. > > Thanks, > > - Greg Pinging the maintainers... This is a benign issue for now but if anyone ever tried to use rpl_nf_ct_tmpl_free in two different modules they'd get a linker error. That probably won't happen but it's still not correct. Thanks, - Greg
On Tue, Nov 05, 2019 at 05:03:56PM -0800, Gregory Rose wrote: > > On 11/5/2019 3:14 PM, Ben Pfaff wrote: > > On Tue, Nov 05, 2019 at 02:14:24PM -0800, Greg Rose wrote: > > > The missing inline keyword before the definition of the > > > rpl_nf_ct_tmpl_free() function causes spurious warnings about the > > > function not being used on some older kernels. Add the keyword > > > to suppress the warning. > > > > > > Signed-off-by: Greg Rose <gvrose8192@gmail.com> > > The commit message and the patch make sense given my experience of GCC > > behavior. I haven't tested it. > > > > Acked-by: Ben Pfaff <blp@ovn.org> > > I only did a compile test of it on Travis. > > https://travis-ci.org/gvrose8192/ovs-experimental/builds/607794710 > > There's one error but it's a resource or infrastructure issue I think. I applied it to master.
On Wed, Nov 13, 2019 at 09:23:02AM -0800, Gregory Rose wrote: > > On 11/5/2019 5:03 PM, Gregory Rose wrote: > > > > On 11/5/2019 3:14 PM, Ben Pfaff wrote: > > > On Tue, Nov 05, 2019 at 02:14:24PM -0800, Greg Rose wrote: > > > > The missing inline keyword before the definition of the > > > > rpl_nf_ct_tmpl_free() function causes spurious warnings about the > > > > function not being used on some older kernels. Add the keyword > > > > to suppress the warning. ... > This is a benign issue for now but if anyone ever tried to use > rpl_nf_ct_tmpl_free in two different modules they'd > get a linker error. That probably won't happen but it's still not correct. Even without "inline", it's still declared "static", which means that it can't duplicate symbols defined in other translation units.
On 11/20/2019 3:26 PM, Ben Pfaff wrote: > On Wed, Nov 13, 2019 at 09:23:02AM -0800, Gregory Rose wrote: >> On 11/5/2019 5:03 PM, Gregory Rose wrote: >>> On 11/5/2019 3:14 PM, Ben Pfaff wrote: >>>> On Tue, Nov 05, 2019 at 02:14:24PM -0800, Greg Rose wrote: >>>>> The missing inline keyword before the definition of the >>>>> rpl_nf_ct_tmpl_free() function causes spurious warnings about the >>>>> function not being used on some older kernels. Add the keyword >>>>> to suppress the warning. > ... > >> This is a benign issue for now but if anyone ever tried to use >> rpl_nf_ct_tmpl_free in two different modules they'd >> get a linker error. That probably won't happen but it's still not correct. > Even without "inline", it's still declared "static", which means that it > can't duplicate symbols defined in other translation units. Well I meant in terms of making it not static. But right, the protection against including the same header content saves us. IMO functions in headers should be inlined but if we're not interested in this patch go ahead and drop it. NBD to me. - Greg
On 11/20/2019 3:24 PM, Ben Pfaff wrote: > On Tue, Nov 05, 2019 at 05:03:56PM -0800, Gregory Rose wrote: >> On 11/5/2019 3:14 PM, Ben Pfaff wrote: >>> On Tue, Nov 05, 2019 at 02:14:24PM -0800, Greg Rose wrote: >>>> The missing inline keyword before the definition of the >>>> rpl_nf_ct_tmpl_free() function causes spurious warnings about the >>>> function not being used on some older kernels. Add the keyword >>>> to suppress the warning. >>>> >>>> Signed-off-by: Greg Rose <gvrose8192@gmail.com> >>> The commit message and the patch make sense given my experience of GCC >>> behavior. I haven't tested it. >>> >>> Acked-by: Ben Pfaff <blp@ovn.org> >> I only did a compile test of it on Travis. >> >> https://travis-ci.org/gvrose8192/ovs-experimental/builds/607794710 >> >> There's one error but it's a resource or infrastructure issue I think. > I applied it to master. OK, thanks.
On Wed, Nov 20, 2019 at 03:47:23PM -0800, Gregory Rose wrote: > > On 11/20/2019 3:26 PM, Ben Pfaff wrote: > > On Wed, Nov 13, 2019 at 09:23:02AM -0800, Gregory Rose wrote: > > > On 11/5/2019 5:03 PM, Gregory Rose wrote: > > > > On 11/5/2019 3:14 PM, Ben Pfaff wrote: > > > > > On Tue, Nov 05, 2019 at 02:14:24PM -0800, Greg Rose wrote: > > > > > > The missing inline keyword before the definition of the > > > > > > rpl_nf_ct_tmpl_free() function causes spurious warnings about the > > > > > > function not being used on some older kernels. Add the keyword > > > > > > to suppress the warning. > > ... > > > > > This is a benign issue for now but if anyone ever tried to use > > > rpl_nf_ct_tmpl_free in two different modules they'd > > > get a linker error. That probably won't happen but it's still not correct. > > Even without "inline", it's still declared "static", which means that it > > can't duplicate symbols defined in other translation units. > Well I meant in terms of making it not static. But right, the protection > against including the same header content saves us. > > IMO functions in headers should be inlined but if we're not interested in > this patch go ahead and drop it. NBD to me. It's the right thing to do, so I committed it.
diff --git a/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h b/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h index bb3d794..4cce92f 100644 --- a/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h +++ b/datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h @@ -33,7 +33,7 @@ out_free: } #define nf_ct_tmpl_alloc rpl_nf_ct_tmpl_alloc -static void rpl_nf_ct_tmpl_free(struct nf_conn *tmpl) +static inline void rpl_nf_ct_tmpl_free(struct nf_conn *tmpl) { nf_ct_ext_destroy(tmpl); nf_ct_ext_free(tmpl);
The missing inline keyword before the definition of the rpl_nf_ct_tmpl_free() function causes spurious warnings about the function not being used on some older kernels. Add the keyword to suppress the warning. Signed-off-by: Greg Rose <gvrose8192@gmail.com> --- datapath/linux/compat/include/net/netfilter/nf_conntrack_core.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)