[next,v2,3/3] batman-adv: make netlink attributes const

Message ID 20160915135249.1925-3-sven@narfation.org (mailing list archive)
State Superseded, archived
Delegated to: Marek Lindner
Headers

Commit Message

Sven Eckelmann Sept. 15, 2016, 1:52 p.m. UTC
  From: stephen hemminger <stephen@networkplumber.org>

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
[sven@narfation.org: Add compat-patch script]
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
v2:
 - new patch picked from the net-next.git

 compat-patches/replacements.sh | 6 ++++++
 net/batman-adv/netlink.c       | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)
  

Comments

Linus Lüssing Oct. 9, 2016, 2:07 a.m. UTC | #1
On Thu, Sep 15, 2016 at 03:52:49PM +0200, Sven Eckelmann wrote:
> +sed -i \
> +	-e 's/^static const struct genl_multicast_group batadv_netlink_mcgrps/static struct genl_multicast_group batadv_netlink_mcgrps/' \
> +	-e 's/^static const struct nla_policy batadv_netlink_policy/static const struct nla_policy batadv_netlink_policy/' \

For the second thing: Replacing the string with itself?


Just wondering... I simply tried casting the const away and that
seems to compile without a warning:

-----
static inline int
 batadv_genl_register_family_with_ops_grps(struct genl_family *family,
                                          struct genl_ops *ops, size_t n_ops,
-                                         struct genl_multicast_group *mcgrps,
+                                         const struct genl_multicast_group *mcgrps,
                                          size_t n_mcgrps)
 {
        family->ops = ops;
        family->n_ops = n_ops;
-       family->mcgrps = mcgrps;
+       family->mcgrps = (struct genl_multicast_group *)mcgrps;
        family->n_mcgrps = n_mcgrps;
        family->module = THIS_MODULE;
-----

Or does this lead to some dangerous behaviour on the compiler side?
  
Sven Eckelmann Oct. 9, 2016, 6:25 a.m. UTC | #2
On Sonntag, 9. Oktober 2016 04:07:10 CEST Linus Lüssing wrote:
> On Thu, Sep 15, 2016 at 03:52:49PM +0200, Sven Eckelmann wrote:
> > +sed -i \
> > +	-e 's/^static const struct genl_multicast_group batadv_netlink_mcgrps/static struct genl_multicast_group batadv_netlink_mcgrps/' \
> > +	-e 's/^static const struct nla_policy batadv_netlink_policy/static const struct nla_policy batadv_netlink_policy/' \
> 
> For the second thing: Replacing the string with itself?

Damn, you are right. This line can be removed anyway. So it is not
"a big problem" but ugly as hell :)

> Just wondering... I simply tried casting the const away and that
> seems to compile without a warning:
[...]
> Or does this lead to some dangerous behaviour on the compiler side?
> 
Yes, this will crash because it is const aka read-only and the compat
code cannot write to the struct (actually, the read-only page).

Kind regards,
	Sven
  

Patch

diff --git a/compat-patches/replacements.sh b/compat-patches/replacements.sh
index c7875c0..974aa96 100755
--- a/compat-patches/replacements.sh
+++ b/compat-patches/replacements.sh
@@ -1,3 +1,9 @@ 
 #! /bin/sh
 
 set -e
+
+# for kernel < 3.13 to make netlink compat code work
+sed -i \
+	-e 's/^static const struct genl_multicast_group batadv_netlink_mcgrps/static struct genl_multicast_group batadv_netlink_mcgrps/' \
+	-e 's/^static const struct nla_policy batadv_netlink_policy/static const struct nla_policy batadv_netlink_policy/' \
+	build/net/batman-adv/netlink.c
diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c
index 18831e7..64cb6ac 100644
--- a/net/batman-adv/netlink.c
+++ b/net/batman-adv/netlink.c
@@ -62,11 +62,11 @@  enum batadv_netlink_multicast_groups {
 	BATADV_NL_MCGRP_TPMETER,
 };
 
-static struct genl_multicast_group batadv_netlink_mcgrps[] = {
+static const struct genl_multicast_group batadv_netlink_mcgrps[] = {
 	[BATADV_NL_MCGRP_TPMETER] = { .name = BATADV_NL_MCAST_GROUP_TPMETER },
 };
 
-static struct nla_policy batadv_netlink_policy[NUM_BATADV_ATTR] = {
+static const struct nla_policy batadv_netlink_policy[NUM_BATADV_ATTR] = {
 	[BATADV_ATTR_VERSION]		= { .type = NLA_STRING },
 	[BATADV_ATTR_ALGO_NAME]		= { .type = NLA_STRING },
 	[BATADV_ATTR_MESH_IFINDEX]	= { .type = NLA_U32 },