[PATCH] powertop: re-use string (~ -36)

Marek Polacek mmpolacek at gmail.com
Wed Nov 3 09:11:11 UTC 2010


On 11/03/2010 02:52 AM, Denys Vlasenko wrote:
> On Wednesday 03 November 2010 00:29, Marek Polacek wrote:
>> Signed-off-by: Marek Polacek <mmpolacek at gmail.com>
>> ---
>>  procps/powertop.c |    5 +++--
>>  1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/procps/powertop.c b/procps/powertop.c
>> index 77b071f..b0bbe7d 100644
>> --- a/procps/powertop.c
>> +++ b/procps/powertop.c
>> @@ -548,6 +548,7 @@ static NOINLINE void print_intel_cstates(void)
>>  	struct dirent *d;
>>  	int i;
>>  	unsigned eax, ebx, ecx, edx;
>> +	static const char *support_str = "Your %s reports these C-states: ";
> 
> Should be static const char support_str[] - you do not need
> to have actual pointer, right?
> 

Of course, why the hell did I do that?  No, we don't need additional
dereference (thus two memory loads).

> You can also use #define. For some strange reason,
> ld merges identical "strings", but not identical
> chans char str[]'s (across modules). Thus #define may fare better.
> 

OK, I'll use #defines.

> 
>>  	cpudir = opendir("/sys/devices/system/cpu");
>>  	if (!cpudir)
>> @@ -616,7 +617,7 @@ static NOINLINE void print_intel_cstates(void)
>>  	if (!edx || !(ecx & 1))
>>  		return;
>>
>> -	printf("Your CPU supports the following C-states: ");
>> +	printf(support_str, "CPU");
>>  	i = 0;
>>  	while (edx) {
>>  		if (edx & 7)
>> @@ -627,7 +628,7 @@ static NOINLINE void print_intel_cstates(void)
>>  	bb_putchar('\n');
>>
>>  	/* Print BIOS C-States */
>> -	printf("Your BIOS reports the following C-states: ");
>> +	printf(support_str, "BIOS");
>>  	for (i = 0; i < ARRAY_SIZE(bios_table); i++)
>>  		if (bios_table[i])
>>  			printf("C%u ", i);
> 
> BTW, the strings were not the same: "reports" != "supports".
> Is it intended that you change them?

Yes, I think their meaning is pretty much the same in this very context.
 But maybe someone else sees it in a different way :).

I'll resend the patch.

Thanks,
			Marek




More information about the busybox mailing list