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?
--
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
