On Thursday 2010-03-25 14:52, Jan Engelhardt wrote:
>On Thursday 2010-03-25 14:27, Patrick McHardy wrote:
>>>>
>>>>> - printk(KERN_WARNING "CLUSTERIP: unknown mode `%u'\n",
>>>>> - cipinfo->hash_mode);
>>>>> + pr_info("unknown mode %u\n", cipinfo->hash_mode);
>>>> pr_err() actually seems more appropriate, if we'd use it consistenly
>>>> to report error conditions.
>>>
>>> I felt that EINVAL parameter problems are not enough of an error
>>> condition to warrant the error level. It's not critical (as in:
>>> printer on fire), error I would associate with sda rejecting I/O,
>>> warning that an NFS server is slow to respond, notice that disk space
>>> is getting below 5% (not that the kernel does that, but that would be
>>> my judgment). The messages printed by checkentry functions is IMO
>>> just an additional information to the -EINVAL that's returned. Of course
>>> we can always change it anyway.
>>
>>Fair enough, but some consistency among modules would be great. There
>>are a few instances of pr_err/warning used for memory allocation errors
>>or invalid parameters in this patch.
>
>As far as I can see, I've been consistent.
>
>pcregrep -r 'pr_(?!info)' net/ipv4/netfilter/ net/ipv6/netfilter/
>net/bridge/netfilter/ net/netfilter/ | grep -v pr_debug |
>grep -v pr_fmt | less
>
>Only shows either lines outside .checkentry, or things that really
>are an error, such as xt_LED.c being unable to register its ledtrigger,
>which is not a "user parameter error" and thus would not be pr_info.
>
>If you see inconsistency, could you hint me towards it?
(Not all patches were initially created using spatch, which I have
just started using yesterday. But it's a great tool.)
So, I was searching for inconsistencies using the following semantic
patch, and the result was only the aforementioned xt_LED.c,
and nf_nat_rule.c (which I'll be fixing up).
// <smpl>
@ rule1 @
struct xt_match match;
identifier mcheck;
@@
match.checkentry = mcheck;
@@
identifier rule1.mcheck;
@@
mcheck(...) {
<...
(
-pr_err
|
-pr_warning
|
-printk
)
+pr_info
(...); ...>
}
@ rule3 @
struct xt_target target;
identifier tcheck;
@@
target.checkentry = tcheck;
@@
identifier rule3.tcheck;
@@
tcheck(...) {
<...
(
-pr_err
|
-pr_warning
|
-printk
)
+pr_info
(...); ...>
}
// </smpl>
--
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
