diff mbox

acpi processor: simplify needlessly complex printk

Message ID 200906282203.55643.elendil@planet.nl (mailing list archive)
State RFC, archived
Headers show

Commit Message

Frans Pop June 28, 2009, 8:03 p.m. UTC
Why use three printk statements if one will do?

Signed-off-by: Frans Pop <elendil@planet.nl>

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Justin P. Mattock June 28, 2009, 10:02 p.m. UTC | #1
Frans Pop wrote:
> Why use three printk statements if one will do?
>
> Signed-off-by: Frans Pop<elendil@planet.nl>
>
> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> index 84e0f3c..b8fb74a 100644
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -772,12 +772,11 @@ static int __cpuinit acpi_processor_start(struct acpi_device *device)
>   	if (result)
>   		printk(KERN_ERR PREFIX "Create sysfs link\n");
>
> -	if (pr->flags.throttling) {
> -		printk(KERN_INFO PREFIX "%s [%s] (supports",
> -		       acpi_device_name(device), acpi_device_bid(device));
> -		printk(" %d throttling states", pr->throttling.state_count);
> -		printk(")\n");
> -	}
> +	if (pr->flags.throttling)
> +		printk(KERN_INFO PREFIX
> +			"%s [%s] (supports %d throttling states)\n",
> +			acpi_device_name(device), acpi_device_bid(device),
> +			pr->throttling.state_count);
>
>         end:
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
>    
Would it be better to capitalize the S and add a period at the
end, or does it not matter?

Justin P. Mattock
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Frans Pop June 28, 2009, 10:28 p.m. UTC | #2
On Monday 29 June 2009, Justin P. Mattock wrote:
> Would it be better to capitalize the S and add a period at the
> end, or does it not matter?

Kernel messages are in general not sentences.

I thought of removing the parenteses, but if you look at the context of 
the message then that only makes sense if other messages are modified at 
the same time. The messages below appear together for my system and their 
consistency could definitely be improved, but IMO that's better done in a 
separate patch.

ACPI: CPU0 (power states: C1[C1] C2[C2] C3[C3])
processor LNXCPU:00: registered as cooling_device7
ACPI: Processor [CPU0] (supports 8 throttling states)
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Justin P. Mattock June 28, 2009, 10:51 p.m. UTC | #3
Frans Pop wrote:
> On Monday 29 June 2009, Justin P. Mattock wrote:
>    
>> Would it be better to capitalize the S and add a period at the
>> end, or does it not matter?
>>      
>
> Kernel messages are in general not sentences.
>
> I thought of removing the parenteses, but if you look at the context of
> the message then that only makes sense if other messages are modified at
> the same time. The messages below appear together for my system and their
> consistency could definitely be improved, but IMO that's better done in a
> separate patch.
>
> ACPI: CPU0 (power states: C1[C1] C2[C2] C3[C3])
> processor LNXCPU:00: registered as cooling_device7
> ACPI: Processor [CPU0] (supports 8 throttling states)
>
>    
Cool, thanks for the info.
I found Documentation/CodingStyle
Chapter 13: Printing kernel messages
Kernel messages do not have to be terminated with a period.
(Sh^t I was going to submit some patches that made proper
some messages, and comments)
I guess I have to learn to change code, instead of messages!

Justin P. Mattock
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 84e0f3c..b8fb74a 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -772,12 +772,11 @@  static int __cpuinit acpi_processor_start(struct acpi_device *device)
 	if (result)
 		printk(KERN_ERR PREFIX "Create sysfs link\n");
 
-	if (pr->flags.throttling) {
-		printk(KERN_INFO PREFIX "%s [%s] (supports",
-		       acpi_device_name(device), acpi_device_bid(device));
-		printk(" %d throttling states", pr->throttling.state_count);
-		printk(")\n");
-	}
+	if (pr->flags.throttling)
+		printk(KERN_INFO PREFIX
+			"%s [%s] (supports %d throttling states)\n",
+			acpi_device_name(device), acpi_device_bid(device),
+			pr->throttling.state_count);
 
       end: