Re: [B.A.T.M.A.N.] [PATCHv14 0/4] Multicast optimizations for bridges

[Sven Eckelmann <sven@narfation.org>]

[-- Attachment #1: Type: text/plain, Size: 3569 bytes --]

On Tuesday 10 May 2016 02:03:11 Linus Lüssing wrote:
> On Thu, May 05, 2016 at 11:15:28PM +0200, Sven Eckelmann wrote:
> > On Thursday 05 May 2016 17:07:01 Linus Lüssing wrote:
> > > This patchset can be found in the current linus/multicast-bridge
> > > branch.
> > 
> > Please check the attached results of the daily build test scripts.
> 
> Thanks Sven!
> 
> Just wanted to note that there seem to be some false positives in
> there.
> 
> "[net/batman-adv/multicast.c:1181]: (style) The function 
'batadv_mcast_flags_seq_print_text' is never used."
> -> used in debugfs.c

Yes, cppcheck has some problems from time to time with it. I can whitelist
this function.

> Also the header suggestions seem a bit unreliable to me,

I have do disagree here. Usually the person using the script or the person 
reading the result it unreliable.

For example I forgot to remove the compat-sources changes from the results. It 
is expected that it doesn't get the headers right here because it builds 
against 4.5 and the changes in it are not for 4.5. So it will not find code 
and thus think that it requires no headers.

> the following removals are false positives, I think:
> 
> * header removals in compat-sources/*
>   -> breaks compiling

See above

 
> * linux/icmp6.h in multicast.c
>   -> icmp6_hdr()
> * linux/ipv6.h in multicast.c
>   -> ipv6_hdr()
> 
> * net/addrconf.h in multicast.c
>   -> ipv6_mc_check_mld()
>   -> ipv6_addr_is_ll_all_nodes()
> * net/ip.h in multicast.c
>   -> ip_eth_mc_map()
> * net/ipv6.h in multicast.c
>   -> IPV6_ADDR_MC_SCOPE
>   -> IPV6_ADDR_SCOPE_LINKLOCAL

Problem seems to be that your sources don't build at the moment. So most 
likely you are missing some dependencies like CONFIG_INET (and maybe even 
more).

The whole thing cannot work when your stuff doesn't build against the headers 
prepared in build_test. I didn't see that you've added something to Kconfig 
and thus I haven't rebuild my headers.

> Moving the following includes upwards seems unnecessary:
> * linux/if_bridge.h in multicast.c
> * linux/igmp.h in multicast.c

This seems to be a side effect of how the script works. Moving is not the 
important part here but just what else is added/removed. But this can be false 
positives because the stuff doesn't build at the moment (even against 4.5)

[...]
> PS: I also do not understand the sparse complaints regarding
> ip_eth_mc_map(). The according include is currently there, seems
> to compile fine.

See above

> I can ignore return type warnings of sparse for the copy & pasted compat
> stuff, right?

No, it will be causing build reports every day. Actually, we wanted no build 
reports when there is no problem (didn't happen since a long time because we 
never were able to submit all patches to net-next.git/net.git)

> Can I ignore the namespace checks mentioned in the end of the log?
> If not, how are they usually resolved for such external functions?

There are usually no external functions. You've introduced them :)

But this is most likely an artifact because you're stuff doesn't compile at 
the moment.

> Is it possible for the scripts to tell why it wants an include for
> linux/kernel.h? Currently can't find any function multicast.c
> might want from it.

I can run it (you can also with the stuff explained some days ago to Antonio 
[1] + checking the "test" file).

It seems to be sprintf

Kind regards,
	Sven

[1] https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2016-May/015203.html

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]