[B.A.T.M.A.N.] [PATCH 1/2] batman-adv: remove forward declaration by including proper header
If main.h is included, the forward declaration for struct batadv_priv is not required.
Cc: Sven Eckelmann sven@narfation.org Signed-off-by: Antonio Quartulli a@unstable.cc ---
Sven, is there any special region for not having the include in this file ? It seems to be compiling just fine.
Cheers,
net/batman-adv/bat_algo.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/bat_algo.h b/net/batman-adv/bat_algo.h index 03dafd3..b727762 100644 --- a/net/batman-adv/bat_algo.h +++ b/net/batman-adv/bat_algo.h @@ -18,7 +18,7 @@ #ifndef _NET_BATMAN_ADV_BAT_ALGO_H_ #define _NET_BATMAN_ADV_BAT_ALGO_H_
-struct batadv_priv; +#include "main.h"
int batadv_iv_init(void);
Some fields in the hard-interface data structure are specific to the B.A.T.M.A.N. V protocol and have to be initialized only when such protocol is compiled in. Instead of having a #ifdef block in the middle of the hard-interface.c code it is better to have an algorithm private function that hides the precompiler logic in its own header file (like other functions).
Fixes: ffd2f27908e5 ("batman-adv: Only init ELP tweaking options when BATMAN_V is enabled") Signed-off-by: Antonio Quartulli a@unstable.cc --- net/batman-adv/bat_algo.h | 5 +++++ net/batman-adv/bat_v.c | 14 ++++++++++++++ net/batman-adv/hard-interface.c | 10 ++-------- 3 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/net/batman-adv/bat_algo.h b/net/batman-adv/bat_algo.h index b727762..3654296 100644 --- a/net/batman-adv/bat_algo.h +++ b/net/batman-adv/bat_algo.h @@ -25,6 +25,7 @@ int batadv_iv_init(void); #ifdef CONFIG_BATMAN_ADV_BATMAN_V
int batadv_v_init(void); +void batadv_v_hardif_init(struct batadv_hard_iface *hardif); int batadv_v_mesh_init(struct batadv_priv *bat_priv); void batadv_v_mesh_free(struct batadv_priv *bat_priv);
@@ -35,6 +36,10 @@ static inline int batadv_v_init(void) return 0; }
+static inline void batadv_v_hardif_init(struct batadv_hard_iface *hardif) +{ +} + static inline int batadv_v_mesh_init(struct batadv_priv *bat_priv) { return 0; diff --git a/net/batman-adv/bat_v.c b/net/batman-adv/bat_v.c index b9c2850..fb2d29d 100644 --- a/net/batman-adv/bat_v.c +++ b/net/batman-adv/bat_v.c @@ -333,6 +333,20 @@ static struct batadv_algo_ops batadv_batman_v __read_mostly = { };
/** + * batadv_v_hardif_init - initialize the algorithm specific fields in the + * hard-interface object + * @hard_iface: the hard-interface to initialize + */ +void batadv_v_hardif_init(struct batadv_hard_iface *hard_iface) +{ + /* enable link throughput auto-detection by setting the throughput + * override to zero + */ + atomic_set(&hard_iface->bat_v.throughput_override, 0); + atomic_set(&hard_iface->bat_v.elp_interval, 500); +} + +/** * batadv_v_mesh_init - initialize the B.A.T.M.A.N. V private resources for a * mesh * @bat_priv: the object representing the mesh interface to initialise diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index a8cda76..a757229 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -15,6 +15,7 @@ * along with this program; if not, see http://www.gnu.org/licenses/. */
+#include "bat_algo.h" #include "hard-interface.h" #include "main.h"
@@ -683,14 +684,7 @@ batadv_hardif_add_interface(struct net_device *net_dev) if (batadv_is_wifi_netdev(net_dev)) hard_iface->num_bcasts = BATADV_NUM_BCASTS_WIRELESS;
-#ifdef CONFIG_BATMAN_ADV_BATMAN_V - /* enable link throughput auto-detection by setting the throughput - * override to zero - */ - atomic_set(&hard_iface->bat_v.throughput_override, 0); - - atomic_set(&hard_iface->bat_v.elp_interval, 500); -#endif + batadv_v_hardif_init(hard_iface);
/* extra reference for return */ kref_init(&hard_iface->refcount);
On Wednesday 11 May 2016 18:29:17 Antonio Quartulli wrote:
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index a8cda76..a757229 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -15,6 +15,7 @@
- along with this program; if not, see http://www.gnu.org/licenses/.
*/
+#include "bat_algo.h" #include "hard-interface.h" #include "main.h"
Can you please use the same order as described in
https://git.open-mesh.org/batman-adv.git/commit/7df6200927271862dea5b66e24b2...
#include "hard-interface.h" #include "main.h"
#include <linux/atomic.h> #include <linux/bug.h> #include <linux/byteorder/generic.h> #include <linux/errno.h> #include <linux/fs.h> #include <linux/if_arp.h> #include <linux/if_ether.h> #include <linux/if.h> #include <linux/kernel.h> #include <linux/kref.h> #include <linux/list.h> #include <linux/netdevice.h> #include <linux/printk.h> #include <linux/rculist.h> #include <linux/rtnetlink.h> #include <linux/slab.h> #include <linux/spinlock.h> #include <linux/workqueue.h>
+#include "bat_algo.h" #include "bridge_loop_avoidance.h" #include "debugfs.h" #include "distributed-arp-table.h" #include "gateway_client.h" #include "originator.h" #include "packet.h" #include "send.h" #include "soft-interface.h" #include "sysfs.h" #include "translation-table.h"
Thanks, Sven
On Wed, May 11, 2016 at 03:11:53PM +0200, Sven Eckelmann wrote:
On Wednesday 11 May 2016 18:29:17 Antonio Quartulli wrote:
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c index a8cda76..a757229 100644 --- a/net/batman-adv/hard-interface.c +++ b/net/batman-adv/hard-interface.c @@ -15,6 +15,7 @@
- along with this program; if not, see http://www.gnu.org/licenses/.
*/
+#include "bat_algo.h" #include "hard-interface.h" #include "main.h"
Can you please use the same order as described in
https://git.open-mesh.org/batman-adv.git/commit/7df6200927271862dea5b66e24b2...
Oh sure I can. Thanks for pointing this out !
Cheers,
On Wednesday 11 May 2016 18:29:16 Antonio Quartulli wrote:
If main.h is included, the forward declaration for struct batadv_priv is not required.
Cc: Sven Eckelmann sven@narfation.org Signed-off-by: Antonio Quartulli a@unstable.cc
Sven, is there any special region for not having the include in this file ? It seems to be compiling just fine.
Cheers,
We have some files which don't include main.h:
* net/batman-adv/bat_algo.h * net/batman-adv/bat_v_ogm.h * net/batman-adv/netlink.h
There is a special exception which should not include it to avoid cycles in the includes:
* net/batman-adv/packet.h
The reason for the other three is just... there is no reason I can provide other than it was not necessary for these files :). But if you want that to have "main.h" included everywhere then please do it really everywhere:
diff --git a/net/batman-adv/bat_algo.h b/net/batman-adv/bat_algo.h index 03dafd3..b727762 100644 --- a/net/batman-adv/bat_algo.h +++ b/net/batman-adv/bat_algo.h @@ -18,7 +18,7 @@ #ifndef _NET_BATMAN_ADV_BAT_ALGO_H_ #define _NET_BATMAN_ADV_BAT_ALGO_H_
-struct batadv_priv; +#include "main.h"
int batadv_iv_init(void);
diff --git a/net/batman-adv/bat_v_ogm.h b/net/batman-adv/bat_v_ogm.h index d849c75..4c4d45c 100644 --- a/net/batman-adv/bat_v_ogm.h +++ b/net/batman-adv/bat_v_ogm.h @@ -18,10 +18,10 @@ #ifndef _BATMAN_ADV_BATADV_V_OGM_H_ #define _BATMAN_ADV_BATADV_V_OGM_H_
+#include "main.h" + #include <linux/types.h>
-struct batadv_hard_iface; -struct batadv_priv; struct sk_buff;
int batadv_v_ogm_init(struct batadv_priv *bat_priv); diff --git a/net/batman-adv/netlink.h b/net/batman-adv/netlink.h index fa152a8..39044cc 100644 --- a/net/batman-adv/netlink.h +++ b/net/batman-adv/netlink.h @@ -18,6 +18,8 @@ #ifndef _NET_BATMAN_ADV_NETLINK_H_ #define _NET_BATMAN_ADV_NETLINK_H_
+#include "main.h" + void batadv_netlink_register(void); void batadv_netlink_unregister(void);
Kind regards, Sven
On Wed, May 11, 2016 at 12:38:31PM +0200, Sven Eckelmann wrote:
On Wednesday 11 May 2016 18:29:16 Antonio Quartulli wrote:
If main.h is included, the forward declaration for struct batadv_priv is not required.
Cc: Sven Eckelmann sven@narfation.org Signed-off-by: Antonio Quartulli a@unstable.cc
Sven, is there any special region for not having the include in this file ? It seems to be compiling just fine.
Cheers,
We have some files which don't include main.h:
- net/batman-adv/bat_algo.h
- net/batman-adv/bat_v_ogm.h
- net/batman-adv/netlink.h
There is a special exception which should not include it to avoid cycles in the includes:
- net/batman-adv/packet.h
The reason for the other three is just... there is no reason I can provide other than it was not necessary for these files :). But if you want that to have "main.h" included everywhere then please do it really everywhere:
Maybe I could include it only when really needed, i.e. * net/batman-adv/bat_algo.h * net/batman-adv/bat_v_ogm.h
(netlink.h does not seem to be requiring it)
Cheers,
On Wednesday 11 May 2016 18:47:30 Antonio Quartulli wrote: [...]
Maybe I could include it only when really needed, i.e.
- net/batman-adv/bat_algo.h
- net/batman-adv/bat_v_ogm.h
(netlink.h does not seem to be requiring it)
This is rather vague and hard to detect automatically. Potentially all files may require it because it has things like:
/* Debug Messages */ #ifdef pr_fmt #undef pr_fmt #endif /* Append 'batman-adv: ' before kernel messages */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
Btw. Just looked at this file again and I should really consider redoing/rebasing the patches [1,2] from Markus Pargmann to clean it up.
Kind regards, Sven
[1] https://patchwork.open-mesh.org/patch/4264/ [2] https://patchwork.open-mesh.org/patch/4244/
On Wed, May 11, 2016 at 02:24:04PM +0200, Sven Eckelmann wrote:
On Wednesday 11 May 2016 18:47:30 Antonio Quartulli wrote: [...]
Maybe I could include it only when really needed, i.e.
- net/batman-adv/bat_algo.h
- net/batman-adv/bat_v_ogm.h
(netlink.h does not seem to be requiring it)
This is rather vague and hard to detect automatically. Potentially all files may require it because it has things like:
/* Debug Messages */ #ifdef pr_fmt #undef pr_fmt #endif /* Append 'batman-adv: ' before kernel messages */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
ok, how about adding it to any header file than (which is probably what you were suggesting in the first place)?
Cheers,
On Friday 13 May 2016 05:07:18 Antonio Quartulli wrote: [...]
ok, how about adding it to any header file than (which is probably what you were suggesting in the first place)?
Yes, this was my first suggestion. Just packet.h and main.h have to be left out. I can also add a check to the daily builds (I have it on my list since your last mail).
Kind regards, Sven
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Sven Eckelmann