diff mbox

[1/2] batman-adv: remove forward declaration by including proper header

Message ID 2789203.91ASHWIDJV@bentobox (mailing list archive)
State Not Applicable, archived
Headers

Commit Message

Sven Eckelmann May 11, 2016, 10:38 a.m. UTC 
  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:



Kind regards,
	Sven

Comments

Antonio Quartulli May 11, 2016, 10:47 a.m. UTC | #1 
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,
Sven Eckelmann May 11, 2016, 12:24 p.m. UTC | #2 
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/
Antonio Quartulli May 12, 2016, 9:07 p.m. UTC | #3 
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,
Sven Eckelmann May 12, 2016, 10:45 p.m. UTC | #4 
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
diff mbox

Patch

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