Skip to contentSkip to conversation

Clean the ip_sockglue.c from some ugly ifdefs

PATCH Applied Pavel Emelyanov 3 participants 4 messages Nov 1, 2007 – Nov 6, 2007

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

Pavel Emelyanov

author
#0 lore raw

PATCH 1/1 Clean the ip_sockglue.c from some ugly ifdefs

The #idfed CONFIG_IP_MROUTE is sometimes places inside the if-s, which looks completely bad. Similar ifdefs inside the functions looks a bit better, but they are also not recommended to be used.

Provide an ifdef-ed ip_mroute_opt() helper to cleanup the code.

Signed-off-by Pavel Emelyanov <[email protected]>

---

include/linux/mroute.h +12 -0
index 7da2cee..200fbb2 100644
@@ -128,6 +128,18 @@ struct igmpmsg
 #ifdef __KERNEL__
 #include <net/sock.h>
 
+#ifdef CONFIG_IP_MROUTE
+static inline int ip_mroute_opt(int opt)
+{
+	return (opt >= MRT_BASE) && (op <= MRT_BASE + 10);
+}
+#else
+static inline int ip_mroute_opt(int opt)
+{
+	return 0;
+}
+#endif
+
 extern int ip_mroute_setsockopt(struct sock *, int, char __user *, int);
 extern int ip_mroute_getsockopt(struct sock *, int, char __user *, int __user *);
 extern int ipmr_ioctl(struct sock *sk, int cmd, void __user *arg);
net/ipv4/ip_sockglue.c +12 -27
index f51f20e..82817e5 100644
@@ -437,10 +437,8 @@ static int do_ip_setsockopt(struct sock *sk, int level,
 
 	/* If optlen==0, it is equivalent to val == 0 */
 
-#ifdef CONFIG_IP_MROUTE
-	if (optname >= MRT_BASE && optname <= (MRT_BASE + 10))
+	if (ip_mroute_opt(optname))
 		return ip_mroute_setsockopt(sk,optname,optval,optlen);
-#endif
 
 	err = 0;
 	lock_sock(sk);

@@ -909,11 +907,9 @@ int ip_setsockopt(struct sock *sk, int level,
 #ifdef CONFIG_NETFILTER
 	/* we need to exclude all possible ENOPROTOOPTs except default case */
 	if (err == -ENOPROTOOPT && optname != IP_HDRINCL &&
-		optname != IP_IPSEC_POLICY && optname != IP_XFRM_POLICY
-#ifdef CONFIG_IP_MROUTE
-		&& (optname < MRT_BASE || optname > (MRT_BASE + 10))
-#endif
-	   ) {
+			optname != IP_IPSEC_POLICY &&
+			optname != IP_XFRM_POLICY &&
+			!ip_mroute_opt(optname)) {
 		lock_sock(sk);
 		err = nf_setsockopt(sk, PF_INET, optname, optval, optlen);
 		release_sock(sk);

@@ -935,11 +931,9 @@ int compat_ip_setsockopt(struct sock *sk, int level, int optname,
 #ifdef CONFIG_NETFILTER
 	/* we need to exclude all possible ENOPROTOOPTs except default case */
 	if (err == -ENOPROTOOPT && optname != IP_HDRINCL &&
-	    optname != IP_IPSEC_POLICY && optname != IP_XFRM_POLICY
-#ifdef CONFIG_IP_MROUTE
-	    && (optname < MRT_BASE || optname > (MRT_BASE + 10))
-#endif
-	   ) {
+			optname != IP_IPSEC_POLICY &&
+			optname != IP_XFRM_POLICY &&
+			!ip_mroute_opt(optname)) {
 		lock_sock(sk);
 		err = compat_nf_setsockopt(sk, PF_INET, optname,
 					   optval, optlen);

@@ -967,11 +961,8 @@ static int do_ip_getsockopt(struct sock *sk, int level, int optname,
 	if (level != SOL_IP)
 		return -EOPNOTSUPP;
 
-#ifdef CONFIG_IP_MROUTE
-	if (optname >= MRT_BASE && optname <= MRT_BASE+10) {
+	if (ip_mroute_opt(optname))
 		return ip_mroute_getsockopt(sk,optname,optval,optlen);
-	}
-#endif
 
 	if (get_user(len,optlen))
 		return -EFAULT;

@@ -1171,11 +1162,8 @@ int ip_getsockopt(struct sock *sk, int level,
 	err = do_ip_getsockopt(sk, level, optname, optval, optlen);
 #ifdef CONFIG_NETFILTER
 	/* we need to exclude all possible ENOPROTOOPTs except default case */
-	if (err == -ENOPROTOOPT && optname != IP_PKTOPTIONS
-#ifdef CONFIG_IP_MROUTE
-		&& (optname < MRT_BASE || optname > MRT_BASE+10)
-#endif
-	   ) {
+	if (err == -ENOPROTOOPT && optname != IP_PKTOPTIONS &&
+			!ip_mroute_opt(optname)) {
 		int len;
 
 		if (get_user(len,optlen))

@@ -1200,11 +1188,8 @@ int compat_ip_getsockopt(struct sock *sk, int level, int optname,
 	int err = do_ip_getsockopt(sk, level, optname, optval, optlen);
 #ifdef CONFIG_NETFILTER
 	/* we need to exclude all possible ENOPROTOOPTs except default case */
-	if (err == -ENOPROTOOPT && optname != IP_PKTOPTIONS
-#ifdef CONFIG_IP_MROUTE
-	    && (optname < MRT_BASE || optname > MRT_BASE+10)
-#endif
-	   ) {
+	if (err == -ENOPROTOOPT && optname != IP_PKTOPTIONS &&
+			!ip_mroute_opt(optname)) {
 		int len;
 
 		if (get_user(len, optlen))
Signature · 1 line
1.5.3.4
#1 lore raw

Em Thu, Nov 01, 2007 at 06:52:34PM +0300, Pavel Emelyanov escreveu:

Quoted 7 lines

The #idfed CONFIG_IP_MROUTE is sometimes places inside the if-s, which looks completely bad. Similar ifdefs inside the functions looks a bit better, but they are also not recommended to be used.

Provide an ifdef-ed ip_mroute_opt() helper to cleanup the code.

Signed-off-by Pavel Emelyanov <[email protected]>

Perhaps a better name would be ip_mroute_valid_opt()?

- Arnaldo

author
#2 lore raw
Quoted 10 lines · Arnaldo Carvalho de Melo

Em Thu, Nov 01, 2007 at 06:52:34PM +0300, Pavel Emelyanov escreveu:

Quoted 7 lines

The #idfed CONFIG_IP_MROUTE is sometimes places inside the if-s, which looks completely bad. Similar ifdefs inside the functions looks a bit better, but they are also not recommended to be used.

Provide an ifdef-ed ip_mroute_opt() helper to cleanup the code.

Signed-off-by Pavel Emelyanov <[email protected]>

Perhaps a better name would be ip_mroute_valid_opt()?

No :) The _valid_ mrote opts are from 0 to 8, according to MRT_XXX macros, not from 0 to 10 as checked. I suspect this was a kind of reserve for future use and thus do not change this.

Correct me if I am wrong.

Quoted 1 line

- Arnaldo

Thanks,
Pavel

maintainer Applied
#3 lore raw
In reply to Pavel Emelyanov

From: Pavel Emelyanov <[email protected]>
Date: Thu, 01 Nov 2007 18:52:34 +0300

Quoted 7 lines

The #idfed CONFIG_IP_MROUTE is sometimes places inside the if-s, which looks completely bad. Similar ifdefs inside the functions looks a bit better, but they are also not recommended to be used.

Provide an ifdef-ed ip_mroute_opt() helper to cleanup the code.

Signed-off-by Pavel Emelyanov <[email protected]>

Applied, thanks Pavel.