Re: [PATCH] kernel/panic.c: reduce 1 byte usage for print tainted buffer.

[Chen Gang <gang.chen@asianux.com>]

On 10/08/2013 08:27 AM, Andrew Morton wrote:
> On Sat, 05 Oct 2013 23:50:39 +0800 Chen Gang <gang.chen@asianux.com> wrote:
> 
>> sizeof("Tainted: ") already counts '\0', and after first sprintf(), 's'
>> will start from the current string end (its' value is '\0').
>>
>> So need not add additional 1 byte for maximized usage of 'buf' in
>> print_tainted().
>>
>>
>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>> ---
>>  kernel/panic.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/kernel/panic.c b/kernel/panic.c
>> index b6c482c..c00b4ce 100644
>> --- a/kernel/panic.c
>> +++ b/kernel/panic.c
>> @@ -233,7 +233,7 @@ static const struct tnt tnts[] = {
>>   */
>>  const char *print_tainted(void)
>>  {
>> -	static char buf[ARRAY_SIZE(tnts) + sizeof("Tainted: ") + 1];
>> +	static char buf[ARRAY_SIZE(tnts) + sizeof("Tainted: ")];
>>  
>>  	if (tainted_mask) {
>>  		char *s;
> 
> hm, that code is a bit crufty.
> 
> - Why is `buf' static?  It could be on-stack.
> 

Just like Al Viro's reply ('buf' will be returned to caller).


> - Requires that the two literal "Tainted: " strings be kept in sync.
> 

Hmm... if more than 2, we should use a macro instead of, else (within
2), especially they are near by, we can still let it hard coded, I feel
that will more straightful for readers and writers.


> - Assumes that strlen("Not tainted") <= strlen("Tainted") +
>   ARRAY_SIZE(tnts).  Which is true, but might not be if someone makes
>   changes...
> 

Hmm... it use snprintf() instead of sprintf(), although I feel better
using scnprintf() instead of.

This string can be trucated, and scnprintf() is more suitable for this
kind of string. And snprintf() is for the string which can not be
truncated (will return the ideal length to notify the caller).

> 
> 
> 

Thanks.
-- 
Chen Gang