[ovs-dev] compat: Add missing inline keyword
diff mbox series

Message ID 1572992064-2249-1-git-send-email-gvrose8192@gmail.com
State New
Headers show
Series
  • [ovs-dev] compat: Add missing inline keyword
Related show

Commit Message

Greg Rose Nov. 5, 2019, 10:14 p.m. UTC
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(-)

Comments

Ben Pfaff Nov. 5, 2019, 11:14 p.m. UTC | #1
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>
Greg Rose Nov. 6, 2019, 1:03 a.m. UTC | #2
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
Greg Rose Nov. 13, 2019, 5:23 p.m. UTC | #3
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
Ben Pfaff Nov. 20, 2019, 11:24 p.m. UTC | #4
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.
Ben Pfaff Nov. 20, 2019, 11:26 p.m. UTC | #5
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.
Greg Rose Nov. 20, 2019, 11:47 p.m. UTC | #6
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
Greg Rose Nov. 20, 2019, 11:48 p.m. UTC | #7
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.
Ben Pfaff Nov. 21, 2019, 2:03 a.m. UTC | #8
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.

Patch
diff mbox series

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);