With a recent kernel you can easily run into a segfault, due to
too big /proc/config.gz:

Configlines: 5000
Line: CONFIG_HAVE_KVM=y

Configlines: 1179537219
Line: CONFIG_HAVE_KVM_IRQCHIP=y

Segmentation fault (core dumped)


Backtrace:
#0  0x00007f1c97030c76 in strcpy () from /lib64/libc.so.6
#1  0x00000000004051a6 in read_kernel_config (configlines=<value optimized 
out>) at config.c:104
#2  suggest_kernel_configs (configlines=<value optimized out>) at config.c:115


This patch moves and renames some functions, so that the configlines
variable can be local. Increase the array to 10000, this should be enough for
a while.

Signed-off-by: Thomas Renninger <[email protected]>

---
 config.c   |   79 ++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 powertop.c |   28 ---------------------
 powertop.h |    2 -
 3 files changed, 66 insertions(+), 43 deletions(-)

Index: powertop/config.c
===================================================================
--- powertop.orig/config.c
+++ powertop/config.c
@@ -32,32 +32,32 @@
 
 #include "powertop.h"
 
-/* static arrays are not nice programming.. but they're easy */
-static char configlines[5000][100];
-static int configcount;
+struct config_t {
+       int configcount;
+       char configlines[10000][100];
+};
 
 /*
  * Suggest the user to turn on/off a kernel config option.
  * "comment" gets displayed if it's not already set to the right value 
  */
-void suggest_kernel_config(char *string, int onoff, char *comment, int weight)
+static void suggest_kernel_config(char *string, int onoff, char *comment,
+                                 int weight, struct config_t cl)
 {
        int i;
        char searchon[100];
        char searchoff[100];
        int found = 0;
 
-       read_kernel_config();
-
        sprintf(searchon, "%s=", string);
        sprintf(searchoff, "# %s is not set", string);
 
-       for (i = 0; i < configcount; i++) {
-               if (onoff && strstr(configlines[i], searchon))
+       for (i = 0; i < cl.configcount; i++) {
+               if (onoff && strstr(cl.configlines[i], searchon))
                        return;
-               if (onoff==0 && strstr(configlines[i], searchoff))
+               if (onoff==0 && strstr(cl.configlines[i], searchoff))
                        return;
-               if (onoff==0 && strstr(configlines[i], searchon))
+               if (onoff==0 && strstr(cl.configlines[i], searchon))
                        found = 1;
        }
        if (onoff || found)
@@ -65,12 +65,13 @@ void suggest_kernel_config(char *string,
        fflush(stdout);
 }
 
-static void read_kernel_config(void)
+static void read_kernel_config(struct config_t *cl)
 {
        FILE *file;
        char version[100], *c;
        char filename[100];
-       if (configcount)
+
+       if (cl->configcount)
                return;
        if (access("/proc/config.gz", R_OK) == 0) {
                file = popen("zcat /proc/config.gz 2> /dev/null", "r");
@@ -78,7 +79,7 @@ static void read_kernel_config(void)
                        char line[100];
                        if (fgets(line, 100, file) == NULL)
                                break;
-                       strcpy(configlines[configcount++], line);
+                       strcpy(cl->configlines[cl->configcount++], line);
                }
                pclose(file);
                return;
@@ -106,7 +107,57 @@ static void read_kernel_config(void)
                char line[100];
                if (fgets(line, 100, file) == NULL)
                        break;
-               strcpy(configlines[configcount++], line);
+               strcpy(cl->configlines[cl->configcount++], line);
        }
        fclose(file);
 }
+
+void suggest_kernel_configs(void)
+{
+       /* arrays are not nice programming.. but they're easy */
+       struct config_t configlines = {0, {}};
+
+       read_kernel_config(&configlines);
+
+       suggest_kernel_config("CONFIG_USB_SUSPEND", 1,
+                             _("Suggestion: Enable the CONFIG_USB_SUSPEND 
kernel configuration option.\n"
+                               "This option will automatically disable UHCI 
USB when not in use, and may\n"
+                               "save approximately 1 Watt of power."), 20, 
configlines);
+
+       suggest_kernel_config("CONFIG_CPU_FREQ_GOV_ONDEMAND", 1,
+                             _("Suggestion: Enable the 
CONFIG_CPU_FREQ_GOV_ONDEMAND kernel configuration option.\n"
+                               "The 'ondemand' CPU speed governor will 
minimize the CPU power usage while\n"
+                               "giving you performance when it is needed."), 
5, configlines);
+
+       suggest_kernel_config("CONFIG_NO_HZ", 1, _("Suggestion: Enable the 
CONFIG_NO_HZ kernel configuration option.\n"
+                                                  "This option is required to 
get any kind of longer sleep times in the CPU."), 50, configlines);
+
+       suggest_kernel_config("CONFIG_ACPI_BATTERY", 1, _("Suggestion: Enable 
the CONFIG_ACPI_BATTERY kernel configuration option.\n "
+                                                         "This option is 
required to get power estimages from PowerTOP"), 5, configlines);
+
+       suggest_kernel_config("CONFIG_HPET_TIMER", 1,
+                             _("Suggestion: Enable the CONFIG_HPET_TIMER 
kernel configuration option.\n"
+                               "Without HPET support the kernel needs to wake 
up every 20 milliseconds for \n"
+                               "some housekeeping tasks."), 10, configlines);
+
+       if (!access("/sys/module/snd_ac97_codec", F_OK) &&
+           access("/sys/module/snd_ac97_codec/parameters/power_save", F_OK))
+               suggest_kernel_config("CONFIG_SND_AC97_POWER_SAVE", 1,
+                                     _("Suggestion: Enable the 
CONFIG_SND_AC97_POWER_SAVE kernel configuration option.\n"
+                                       "This option will automatically power 
down your sound codec when not in use,\n"
+                                       "and can save approximately half a Watt 
of power."), 20, configlines);
+
+       suggest_kernel_config("CONFIG_IRQBALANCE", 0,
+                             _("Suggestion: Disable the CONFIG_IRQBALANCE 
kernel configuration option.\n"
+                               "The in-kernel irq balancer is obsolete and 
wakes the CPU up far more than needed."), 3, configlines);
+
+       suggest_kernel_config("CONFIG_CPU_FREQ_STAT", 1,
+                             _("Suggestion: Enable the CONFIG_CPU_FREQ_STAT 
kernel configuration option.\n"
+                               "This option allows PowerTOP to show P-state 
percentages \n"
+                               "P-states correspond to CPU frequencies."), 2, 
configlines);
+
+       suggest_kernel_config("CONFIG_INOTIFY", 1,
+                             _("Suggestion: Enable the CONFIG_INOTIFY kernel 
configuration option.\n"
+                               "This option allows programs to wait for 
changes in files and directories\n"
+                               "instead of having to poll for these changes"), 
5, configlines);
+}
Index: powertop/powertop.c
===================================================================
--- powertop.orig/powertop.c
+++ powertop/powertop.c
@@ -1122,34 +1122,6 @@ int main(int argc, char **argv)
 
                reset_suggestions();
 
-               suggest_kernel_config("CONFIG_USB_SUSPEND", 1,
-                                   _("Suggestion: Enable the 
CONFIG_USB_SUSPEND kernel configuration option.\nThis option will automatically 
disable UHCI USB when not in use, and may\nsave approximately 1 Watt of 
power."), 20);
-               suggest_kernel_config("CONFIG_CPU_FREQ_GOV_ONDEMAND", 1,
-                                   _("Suggestion: Enable the 
CONFIG_CPU_FREQ_GOV_ONDEMAND kernel configuration option.\n"
-                                     "The 'ondemand' CPU speed governor will 
minimize the CPU power usage while\n" "giving you performance when it is 
needed."), 5);
-               suggest_kernel_config("CONFIG_NO_HZ", 1, _("Suggestion: Enable 
the CONFIG_NO_HZ kernel configuration option.\nThis option is required to get 
any kind of longer sleep times in the CPU."), 50);
-               suggest_kernel_config("CONFIG_ACPI_BATTERY", 1, _("Suggestion: 
Enable the CONFIG_ACPI_BATTERY kernel configuration option.\n "
-                                     "This option is required to get power 
estimages from PowerTOP"), 5);
-               suggest_kernel_config("CONFIG_HPET_TIMER", 1,
-                                   _("Suggestion: Enable the CONFIG_HPET_TIMER 
kernel configuration option.\n"
-                                     "Without HPET support the kernel needs to 
wake up every 20 milliseconds for \n" "some housekeeping tasks."), 10);
-               if (!access("/sys/module/snd_ac97_codec", F_OK) &&
-                       
access("/sys/module/snd_ac97_codec/parameters/power_save", F_OK))
-                       suggest_kernel_config("CONFIG_SND_AC97_POWER_SAVE", 1,
-                                   _("Suggestion: Enable the 
CONFIG_SND_AC97_POWER_SAVE kernel configuration option.\n"
-                                     "This option will automatically power 
down your sound codec when not in use,\n"
-                                     "and can save approximately half a Watt 
of power."), 20);
-               suggest_kernel_config("CONFIG_IRQBALANCE", 0,
-                                     _("Suggestion: Disable the 
CONFIG_IRQBALANCE kernel configuration option.\n" "The in-kernel irq balancer 
is obsolete and wakes the CPU up far more than needed."), 3);
-               suggest_kernel_config("CONFIG_CPU_FREQ_STAT", 1,
-                                   _("Suggestion: Enable the 
CONFIG_CPU_FREQ_STAT kernel configuration option.\n"
-                                     "This option allows PowerTOP to show 
P-state percentages \n" "P-states correspond to CPU frequencies."), 2);
-               suggest_kernel_config("CONFIG_INOTIFY", 1,
-                                   _("Suggestion: Enable the CONFIG_INOTIFY 
kernel configuration option.\n"
-                                     "This option allows programs to wait for 
changes in files and directories\n" 
-                                     "instead of having to poll for these 
changes"), 5);
-
-
                /* suggest to stop beagle if it shows up in the top 20 and 
wakes up more than 10 times in the measurement */
                suggest_process_death("beagled : schedule_timeout", "beagled", 
lines, min(linehead,20), 10.0,
                                    _("Suggestion: Disable or remove 'beagle' 
from your system. \n"
Index: powertop/powertop.h
===================================================================
--- powertop.orig/powertop.h
+++ powertop/powertop.h
@@ -46,7 +46,7 @@ extern int             linectotal;
 extern double displaytime;
 
 void suggest_process_death(char *process_match, char *process_name, struct 
line *slines, int linecount, double minwakeups, char *comment, int weight);
-void suggest_kernel_config(char *string, int onoff, char *comment, int weight);
+void suggest_kernel_configs(void);
 void suggest_bluetooth_off(void);
 void suggest_nmi_watchdog(void);
 void suggest_hpet(void);

_______________________________________________
Power mailing list
[email protected]
http://www.bughost.org/mailman/listinfo/power

Reply via email to