Krzysztof Oledzki wrote:
On Fri, 6 Jun 2008, Patrick McHardy wrote:
Looks pretty good, just one or two minor things below. What would
be nice though is to reduce patch size a bit, two suggestions for
this are:
- move seq_print_acct() changes to seperate cleanup patch
But those are not only cleanup changes but changes mainly required by
ct_extend implementation. The only cleanup change is that there is now
one function instead of two identical. Should I first change both
seq_print_counters into seq_print_acct and then merge them in an
additional post-patch?
If it makes sense, sure. Those were just suggestions, if
they don't make sense, feel free to ignore them :)
struct hlist_head *nf_ct_alloc_hashtable(unsigned int *sizep, int
*vmalloced)
@@ -1165,6 +1174,10 @@ int __init nf_conntrack_init(void)
if (ret < 0)
goto out_fini_expect;
+ ret = nf_conntrack_acct_init();
+ if (ret < 0)
+ goto out_fini_acct;
+
/* For use by REJECT target */
rcu_assign_pointer(ip_ct_attach, nf_conntrack_attach);
rcu_assign_pointer(nf_ct_destroy, destroy_conntrack);
@@ -1177,6 +1190,7 @@ int __init nf_conntrack_init(void)
return ret;
+out_fini_acct:
nf_conntrack_helper_fini(); ?
Hm... "helper"? You mean "acct"?
No, I mean helper. If acct initialization fails, the helper
initialization was the last thing that succeeded and you need
to properly clean it up again.
There is no nf_conntrack_acct_fini as
it seems to be currently unnecessary. So, this empty out_fini_acct seems
to be redundant here but I decided that adding it is cleaner than
calling "goto out_fini_*expect*" from the nf_conntrack_*acct*_init
error-path.
Good point, even though accounting and ct_extend are unloaded
together, for cleanness you should also unregister the extension
you've registered.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
