ARC-setup: Use seq_putc() in show_cpuinfo()
diff mbox

Message ID 164a402a-de20-645d-00af-9a414cf745c4@users.sourceforge.net
State New
Headers show

Commit Message

SF Markus Elfring Oct. 15, 2016, 7:56 p.m. UTC
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 15 Oct 2016 21:31:16 +0200

A single character (line break) should be put into a sequence.
Thus use the corresponding function "seq_putc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/arc/kernel/setup.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Vineet Gupta Oct. 17, 2016, 4:25 p.m. UTC | #1
On 10/15/2016 12:56 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 15 Oct 2016 21:31:16 +0200
>
> A single character (line break) should be put into a sequence.
> Thus use the corresponding function "seq_putc".

Perhaps reword the changelog to say that seqc_putc is more efficient than
seqc_printf to output a single char.
I mean _printf is not wrong but not as efficient ?

>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  arch/arc/kernel/setup.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
> index 3df7f9c..e3f5432 100644
> --- a/arch/arc/kernel/setup.c
> +++ b/arch/arc/kernel/setup.c
> @@ -494,8 +494,7 @@ static int show_cpuinfo(struct seq_file *m, void *v)
>  
>  	free_page((unsigned long)str);
>  done:
> -	seq_printf(m, "\n");
> -
> +	seq_putc(m, '\n');
>  	return 0;
>  }
>
SF Markus Elfring Oct. 17, 2016, 5:19 p.m. UTC | #2
>> A single character (line break) should be put into a sequence.
>> Thus use the corresponding function "seq_putc".
> 
> Perhaps reword the changelog to say that seqc_putc is more efficient than
> seqc_printf to output a single char.
> I mean _printf is not wrong but not as efficient ?

I came along source files for a few other software modules with similar
change possibilities.
Unfortunately, the corresponding developers are not convinced yet
to replace a call of the function "seq_printf" at the end by
a "seq_putc" because of software efficiency reasons.

Do you find this update suggestion acceptable to some degree
for the function "setup"?

Regards,
Markus
Vineet Gupta Oct. 17, 2016, 5:23 p.m. UTC | #3
On 10/17/2016 10:19 AM, SF Markus Elfring wrote:
>>> A single character (line break) should be put into a sequence.
>>> Thus use the corresponding function "seq_putc".
>> Perhaps reword the changelog to say that seqc_putc is more efficient than
>> seqc_printf to output a single char.
>> I mean _printf is not wrong but not as efficient ?
> I came along source files for a few other software modules with similar
> change possibilities.
> Unfortunately, the corresponding developers are not convinced yet
> to replace a call of the function "seq_printf" at the end by
> a "seq_putc" because of software efficiency reasons.

I was ambivalent so far - but not anymore :-)
what is the objection - can you point me to a few links where people don't think
this is not a good idea.

> Do you find this update suggestion acceptable to some degree
> for the function "setup"?
>
> Regards,
> Markus
>
SF Markus Elfring Oct. 17, 2016, 5:39 p.m. UTC | #4
>>> Perhaps reword the changelog to say that seqc_putc is more efficient than
>>> seqc_printf to output a single char.
>>> I mean _printf is not wrong but not as efficient ?
>> I came along source files for a few other software modules with similar
>> change possibilities.
>> Unfortunately, the corresponding developers are not convinced yet
>> to replace a call of the function "seq_printf" at the end by
>> a "seq_putc" because of software efficiency reasons.
> 
> I was ambivalent so far - but not anymore :-)

Interesting …


> what is the objection - can you point me to a few links where people don't think
> this is not a good idea.

Yes, of course. - Does the double negation in this wording indicate another
special software development concern?

How do you think about another update suggestion like "[PATCH] MD-RAID: Use seq_putc()
in three status functions" (from 2016-10-16)?
https://patchwork.kernel.org/patch/9378055/
https://lkml.kernel.org/r/<77fb6fdc-7480-8607-0af1-42f73c125b9d@users.sourceforge.net>


>> Do you find this update suggestion acceptable to some degree
>> for the function "setup"?

I am curious what your opinions will be for further development of the
function "show_cpuinfo" in the source file "arch/arc/kernel/setup.c".

Regards,
Markus

Patch
diff mbox

diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
index 3df7f9c..e3f5432 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -494,8 +494,7 @@  static int show_cpuinfo(struct seq_file *m, void *v)
 
 	free_page((unsigned long)str);
 done:
-	seq_printf(m, "\n");
-
+	seq_putc(m, '\n');
 	return 0;
 }