From: David Miller <davem@davemloft.net>
To: den@openvz.org
Cc: benjamin.thery@bull.net, dlezcano@fr.ibm.com, devel@openvz.org,
containers@lists.osdl.org, netdev@vger.kernel.org,
xemul@openvz.org
Subject: Re: [PATCH net-2.6.25 1/19] [NETNS] Add netns parameter to fib_rules_(un)register.
Date: Thu, 20 Dec 2007 15:46:36 -0800 (PST) [thread overview]
Message-ID: <20071220.154636.86238588.davem@davemloft.net> (raw)
In-Reply-To: <1198077889-10693-2-git-send-email-den@openvz.org>
From: "Denis V. Lunev" <den@openvz.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.
next prev parent reply other threads:[~2007-12-20 23:46 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-12-19 15:21 [PATCH netns-2.6.25 0/19] routing virtualization Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 1/19] [NETNS] Add netns parameter to fib_rules_(un)register Denis V. Lunev
2007-12-20 23:46 ` David Miller [this message]
2007-12-19 15:24 ` [PATCH net-2.6.25 2/19] [NETNS] Pass fib_rules_ops into default_pref method Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 3/19] [NETNS] Namespacing in the generic fib rules code Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 4/19] [NETNS] Add namespace to API for routing /proc entries creation Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 5/19] [IPV4] Check fib4_rules_init failure Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 6/19] [NETNS] Refactor fib initialization so it can handle multiple namespaces Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 7/19] [IPV4] Unify access to the routing tables Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 8/19] [NETNS] Add netns parameter to fib_get_table/fib_new_table Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 9/19] [NETNS] Add netns parameter to inet_(dev_)add_type Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 10/19] [NETNS] Add netns to nl_info structure Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 11/19] [NETNS] Show routing information from correct namespace (fib_hash.c) Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 12/19] [NETNS] Show routing information from correct namespace (fib_trie.c) Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 13/19] [NETNS] Namespacing IPv4 fib rules Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 14/19] [NETNS] Place fib tables into netns Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 15/19] [NETNS] Provide correct namespace for fibnl netlink socket Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 16/19] [NETNS] Correctly fill fib_config data Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 17/19] [NETNS] Pass namespace through ip_rt_ioctl Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 18/19] [NETNS] Replace init_net with the correct context in fib_frontend.c Denis V. Lunev
2007-12-19 15:24 ` [PATCH net-2.6.25 19/19] [NETNS] Enable routing configuration in non-initial namespace Denis V. Lunev
-- strict thread matches above, loose matches on Subject: below --
2008-01-09 18:03 [PATCH netns-2.6.25 0/19] routing virtualization v2 Denis V. Lunev
[not found] ` <47850C57.60907-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
2008-01-09 18:04 ` [PATCH net-2.6.25 1/19] [NETNS] Add netns parameter to fib_rules_(un)register Denis V. Lunev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20071220.154636.86238588.davem@davemloft.net \
--to=davem@davemloft.net \
--cc=benjamin.thery@bull.net \
--cc=containers@lists.osdl.org \
--cc=den@openvz.org \
--cc=devel@openvz.org \
--cc=dlezcano@fr.ibm.com \
--cc=netdev@vger.kernel.org \
--cc=xemul@openvz.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).