Rafael.Vanoni at Sun.COM wrote:
> Author: Rafael Vanoni <rafael.vanoni at sun.com>
> Repository: /hg/tesla/powertop
> Latest revision: 9ebc92d0cbac4ad541dbd62fea829f8af4042e81
> Total changesets: 1
> Log message:
> Code review comments from Eric Saxe
>
> Files:
> update: usr/src/cmd/powertop/amd64/pt_amd64.c
> update: usr/src/cmd/powertop/common/cpufreq.c
> update: usr/src/cmd/powertop/common/display.c
> update: usr/src/cmd/powertop/i386/pt_i386.c
> update: usr/src/cmd/powertop/sparcv9/pt_sparcv9.c
> _______________________________________________
> tesla-dev mailing list
> tesla-dev at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/tesla-dev
Had forgotten to push this changeset, it has a handfull of changes after
Eric's code review. Here's a diff:
rv at catfish:/sandbox/powertop/clone$ hg diff -r 8786 -r 8787
diff -r 9aa3ef2a0a53 -r 9ebc92d0cbac usr/src/cmd/powertop/amd64/pt_amd64.c
--- a/usr/src/cmd/powertop/amd64/pt_amd64.c Fri Apr 03 14:28:06 2009 -0700
+++ b/usr/src/cmd/powertop/amd64/pt_amd64.c Fri Apr 03 16:25:25 2009 -0700
@@ -215,5 +215,5 @@
/*
* amd64 platform specific display messages
*/
-const char *g_msg_idle_state = "C-states (idleness)";
+const char *g_msg_idle_state = "C-states (idle power state)";
const char *g_msg_freq_state = "P-states (frequencies)";
diff -r 9aa3ef2a0a53 -r 9ebc92d0cbac usr/src/cmd/powertop/common/cpufreq.c
--- a/usr/src/cmd/powertop/common/cpufreq.c Fri Apr 03 14:28:06 2009 -0700
+++ b/usr/src/cmd/powertop/common/cpufreq.c Fri Apr 03 16:25:25 2009 -0700
@@ -45,6 +45,8 @@
#include "powertop.h"
#define HZ2MHZ(speed) ((speed) / 1000000)
+#define DTP_ARG_COUNT 2
+#define DTP_ARG_LENGTH 5
static uint64_t max_cpufreq = 0;
static dtrace_hdl_t *dtp;
@@ -127,10 +129,10 @@
static int
pt_cpufreq_setup(void)
{
- if ((dtp_argv = malloc(sizeof (char *) * 2)) == NULL)
+ if ((dtp_argv = malloc(sizeof (char *) * DTP_ARG_COUNT)) == NULL)
return (EXIT_FAILURE);
- if ((dtp_argv[0] = malloc(sizeof (char) * 5)) == NULL) {
+ if ((dtp_argv[0] = malloc(sizeof (char) * DTP_ARG_LENGTH)) == NULL) {
free(dtp_argv);
return (EXIT_FAILURE);
}
@@ -138,7 +140,8 @@
(void) snprintf(dtp_argv[0], 5, "%d\0", g_ncpus_observed);
if (PTOP_ON_CPU) {
- if ((dtp_argv[1] = malloc(sizeof (char) * 5)) == NULL) {
+ if ((dtp_argv[1] = malloc(sizeof (char) * DTP_ARG_LENGTH))
+ == NULL) {
free(dtp_argv[0]);
free(dtp_argv);
return (EXIT_FAILURE);
diff -r 9aa3ef2a0a53 -r 9ebc92d0cbac usr/src/cmd/powertop/common/display.c
--- a/usr/src/cmd/powertop/common/display.c Fri Apr 03 14:28:06 2009 -0700
+++ b/usr/src/cmd/powertop/common/display.c Fri Apr 03 16:25:25 2009 -0700
@@ -239,7 +239,7 @@
(void) wbkgd(cstate_window, COLOR_PAIR(PT_COLOR_DEFAULT));
}
- print(cstate_window, 0, 0, "%s\t\tAvg residency\n", g_msg_idle_state);
+ print(cstate_window, 0, 0, "%s\tAvg residency\n", g_msg_idle_state);
res = (((double)g_cstate_info[0].total_time / g_total_c_time)) * 100;
(void) sprintf(c, "C0 (cpu running)\t\t(%.1f%%)\n", (float)res);
print(cstate_window, 1, 0, "%s", c);
@@ -318,13 +318,13 @@
*/
(void) sprintf(c, "%4lu Mhz(turbo)\t%.1f%%",
(long)p0_speed,
- 100 * (g_pstate_info[i].total_time/g_ncpus/1000000
- /total_pstates));
+ 100 * (g_pstate_info[i].total_time/
+ g_ncpus_observed/1000000/total_pstates));
} else {
(void) sprintf(c, "%4lu Mhz\t%.1f%%",
(long)g_pstate_info[i].speed,
- 100 * (g_pstate_info[i].total_time/g_ncpus/1000000
- /total_pstates));
+ 100 * (g_pstate_info[i].total_time/
+ g_ncpus_observed/1000000/total_pstates));
}
print(cstate_window, i+1, 48, "%s\n", c);
}
@@ -356,11 +356,11 @@
break;
case 1:
(void) sprintf(c, "(discharging: %3.1f hours)",
- rem_cap/rate);
+ (uint32_t)rem_cap/rate);
break;
case 2:
(void) sprintf(c, "(charging: %3.1f hours)",
- (cap - rem_cap)/rate);
+ (uint32_t)(cap - rem_cap)/rate);
break;
case 4:
(void) sprintf(c, "(##critically low battery power##)");
diff -r 9aa3ef2a0a53 -r 9ebc92d0cbac usr/src/cmd/powertop/i386/pt_i386.c
--- a/usr/src/cmd/powertop/i386/pt_i386.c Fri Apr 03 14:28:06 2009 -0700
+++ b/usr/src/cmd/powertop/i386/pt_i386.c Fri Apr 03 16:25:25 2009 -0700
@@ -215,5 +215,5 @@
/*
* i386 platform specific display messages
*/
-const char *g_msg_idle_state = "C-states (idleness)";
+const char *g_msg_idle_state = "C-states (idle power state)";
const char *g_msg_freq_state = "P-states (frequencies)";
diff -r 9aa3ef2a0a53 -r 9ebc92d0cbac
usr/src/cmd/powertop/sparcv9/pt_sparcv9.c
--- a/usr/src/cmd/powertop/sparcv9/pt_sparcv9.c Fri Apr 03 14:28:06 2009
-0700
+++ b/usr/src/cmd/powertop/sparcv9/pt_sparcv9.c Fri Apr 03 16:25:25 2009
-0700
@@ -245,6 +245,5 @@
/*
* sparcv9 platform specific display messages
*/
-const char *g_msg_idle_state = "Idle states\t";
-const char *g_msg_freq_state = "Frequency levels";
-
+const char *g_msg_idle_state = "Idle Power states\t";
+const char *g_msg_freq_state = "Frequency levels";
\ No newline at end of file
rv at catfish:/sandbox/powertop/clone$
Rafael