Re: [PATCH 3/5] Add the "verbose" module option.

[Goffredo Baroncelli]

On 08/07/2014 06:43 PM, Jean Delvare wrote:
> On Thu, 07 Aug 2014 18:29:23 +0200, Goffredo Baroncelli wrote:
>> On 08/07/2014 10:52 AM, Jean Delvare wrote:
>>> Le Wednesday 06 August 2014 à 21:05 +0000, Goffredo Baroncelli a écrit :
>>>> +	 */
>>>> +	tempchanged = x.temp != temp || x.casetemp != casetemp;
>>>> +	if ((verbose > 1 && tempchanged) ||
>>>> +	    (verbose > 0 && level >= 0)) {
>>>> +		printk(KERN_INFO);
>>>> +		print_temp("CPU-temp: ", temp);
>>>
>>> This can be written more efficiently as a single statement:
>>>
>>> 		print_temp(KERN_INFO "CPU-temp: ", temp);
>>
>> I suppose that KERN_* has to be in the beginning of the line. 
> 
> Correct.
> 
>> Because a single line is composed by several prink,
> 
> In this case, it is, but FYI, this is generally discouraged. The reason
> is that another piece of the kernel may be calling printk at the same
> time, and then that other message may split your own message into
> pieces. If you run checkpatch.pl on this file, you'll see it complains
> about this.
> 
>> KERN_INFO has 
>> to be only in the first printk. To me it seems more polite to have
>> one printk for the level, and the others (there are more than one) 
>> for the message parts.
> 
> The fewer printks is better. Ideally there would be only one to avoid
> the risk of line splitting altogether. I understand this isn't easy to
> achieve in this case, but I still believe that you shouldn't have more
> calls to printk than necessary, to reduce the risk.
> 
Ok, now I understand the reason. I will remove the first printk.

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/