netdev - Re: [PATCH net-2.6.25 1/19] [NETNS] Add netns parameter to fib_rules

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 20 Dec 2007 15:46:36 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	den@...nvz.org
Cc:	benjamin.thery@...l.net, dlezcano@...ibm.com, devel@...nvz.org,
	containers@...ts.osdl.org, netdev@...r.kernel.org, xemul@...nvz.org
Subject: Re: [PATCH net-2.6.25 1/19] [NETNS] Add netns parameter to
 fib_rules_(un)register.

From: "Denis V. Lunev" <den@...nvz.org>
Date: Wed, 19 Dec 2007 18:24:31 +0300

> @@ -101,14 +101,12 @@ static inline u32 frh_get_table(struct fib_rule_hdr *frh, struct nlattr **nla)
>  	return frh->table;
>  }
>  
> -extern int			fib_rules_register(struct fib_rules_ops *);
> -extern int			fib_rules_unregister(struct fib_rules_ops *);
> -extern void                     fib_rules_cleanup_ops(struct fib_rules_ops *);
> +extern int fib_rules_register(struct net *, struct fib_rules_ops *);
> +extern int fib_rules_unregister(struct net *, struct fib_rules_ops *);
> +extern void fib_rules_cleanup_ops(struct fib_rules_ops *);
>  
> -extern int			fib_rules_lookup(struct fib_rules_ops *,
> -						 struct flowi *, int flags,
> -						 struct fib_lookup_arg *);
> -extern int			fib_default_rule_add(struct fib_rules_ops *,
> -						     u32 pref, u32 table,
> -						     u32 flags);
> +extern int fib_rules_lookup(struct fib_rules_ops *, struct flowi *, int flags,
> +			    struct fib_lookup_arg *);
> +extern int fib_default_rule_add(struct fib_rules_ops *, u32 pref, u32 table,
> +				u32 flags);
>  #endif

Please do not make gratuitous coding style changes like this!

What bothers you so much that there is lots of whitespace there after
the "extern int"?  Does it bother you so much that you think the side
effect of your patch being unreadable is worth it?!?!

Why is it unreadable?  I'm glad you asked....

Just like me, someone will have to read this over carefully to
see what you're actually doing.

Are you deleting all the existing declarations and adding new
ones with different names?

Are you deleting some of them, but keeping others yet changing
the arguments to them somehow?

Are you deleting some of them, but masterbating with the coding
style of others?

NOBODY KNOWS!

Whereas if you just deleted the lines for the functions you
are removing, it would be totally clear what is happening.

This patch, from a reviewability standpoint, sucks.  It makes
efficient patch review next to impossible.

I'm not looking at the rest of this patch set, clean this stuff up and
resubmit it all, thank you.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html