Merge lp:~cmiller/ubuntu/trusty/icewm/translations-cause-crash-lp447883 into lp:ubuntu/trusty/icewm

[Chad Miller]

[Dmitry Shachnev (mitya57)]

Thanks for your work here.

- The patch contains come unrelated changes (like 65536.0 → FLOAT_65K change), please drop that and keep diff minimal;
- Please submit the patch upstream so that we don't have to carry a delta;
- Please add a proper changelog entry (with credits to original patch author).

review: Needs Fixing

Unmerged revisions

19. By Chad Miller on 2013-10-22

Allocate memory that's needed to fill system status info, instead of fragile filing of static allocation.

In particular, some sprintf()s were kind of dumb when the format strings were retrieved from gettext translations. (LP: #447883)

=== modified file '.pc/applied-patches'
--- .pc/applied-patches	2013-06-29 17:59:17 +0000
+++ .pc/applied-patches	2013-10-22 21:19:12 +0000
@@ -14,3 +14,4 @@
 ftbfs-gcc-4.7.diff
 icon_property_fix
 safe_get_argument
+tooltip-crash.patch

=== modified file 'debian/patches/series'
--- debian/patches/series	2013-06-29 17:59:17 +0000
+++ debian/patches/series	2013-10-22 21:19:12 +0000
@@ -15,3 +15,4 @@
 ftbfs-gcc-4.7.diff
 icon_property_fix
 safe_get_argument
+tooltip-crash.patch

=== added file 'debian/patches/tooltip-crash.patch'
--- debian/patches/tooltip-crash.patch	1970-01-01 00:00:00 +0000
+++ debian/patches/tooltip-crash.patch	2013-10-22 21:19:12 +0000
@@ -0,0 +1,189 @@
+Index: icewm/src/acpustatus.cc
+===================================================================
+--- icewm.orig/src/acpustatus.cc	2013-10-22 16:48:27.202406021 -0400
++++ icewm/src/acpustatus.cc	2013-10-22 16:56:24.396772296 -0400
+@@ -213,38 +213,79 @@
+ 
+ void CPUStatus::updateToolTip() {
+ #ifdef linux
+-    char load[31], ram[31] = "", swap[31] = "", acpitemp[61] = "", cpufreq[31] = "";
++    const char LOAD_FORMAT[] = "CPU Load: %3.2f %3.2f %3.2f, %d";
++    const float FLOAT_65K = 65536.0;
++    const float FLOAT_1M = 1048576.0f;
++    char *load = NULL, *ram = NULL, *swap = NULL, *cpufreq = NULL;
++    char acpitemp[61];
+     struct sysinfo sys;
+     double l1, l5, l15;
++    int size;
+ 
+     sysinfo(&sys);
+-    l1 = (float)sys.loads[0] / 65536.0;
+-    l5 = (float)sys.loads[1] / 65536.0;
+-    l15 = (float)sys.loads[2] / 65536.0;
++    l1 = (float)sys.loads[0] / FLOAT_65K;
++    l5 = (float)sys.loads[1] / FLOAT_65K;
++    l15 = (float)sys.loads[2] / FLOAT_65K;
++
++    size = snprintf(NULL, 0, _(LOAD_FORMAT), l1, l5, l15, sys.procs);
++    if (size > 0)
++        load = new char[size + 1];
++    if (load)
++        sprintf(load, _(LOAD_FORMAT), l1, l5, l15, sys.procs);
+ 
+-    sprintf(load, _("CPU Load: %3.2f %3.2f %3.2f, %d"), l1, l5, l15, sys.procs);
+     if (ShowRamUsage) {
+-        float tr =(float)sys.totalram * (float)sys.mem_unit / 1048576.0f;
+-        float fr =(float)sys.freeram * (float)sys.mem_unit / 1048576.0f;
+-        sprintf(ram, _("\nRam: %5.2f/%.2fM"), tr, fr);
++        const char RAM_FORMAT[] = "\nRam: %5.2f/%.2fM";
++        float tr =(float)sys.totalram * (float)sys.mem_unit / FLOAT_1M;
++        float fr =(float)sys.freeram * (float)sys.mem_unit / FLOAT_1M;
++        size = snprintf(NULL, 0, _(RAM_FORMAT), tr, fr);
++        if (size > 0)
++            ram = new char[size + 1];
++        if (ram)
++            sprintf(ram, _(RAM_FORMAT), tr, fr);
+     }
+     if (ShowSwapUsage) {
+-        float ts =(float)sys.totalswap * (float)sys.mem_unit / 1048576.0f;
+-        float fs =(float)sys.freeswap * (float)sys.mem_unit / 1048576.0f;
+-        sprintf(swap, _("\nSwap: %.2f/%.2fM"), ts, fs);
++        const char SWAP_FORMAT[] = "\nSwap: %.2f/%.2fM";
++        float ts =(float)sys.totalswap * (float)sys.mem_unit / FLOAT_1M;
++        float fs =(float)sys.freeswap * (float)sys.mem_unit / FLOAT_1M;
++        size = snprintf(NULL, 0, _(SWAP_FORMAT), ts, fs);
++        if (size > 0)
++            swap = new char[size + 1];
++        if (swap)
++            sprintf(swap, _(SWAP_FORMAT), ts, fs);
+     }
+     if (ShowAcpiTemp) {
+-        int headlen;
+-        sprintf(acpitemp, _("\nACPI Temp:"));
+-        headlen = strlen(acpitemp);
+-        getAcpiTemp(acpitemp + headlen, (sizeof(acpitemp) - headlen) / sizeof(char));
++        size = snprintf(acpitemp, sizeof(acpitemp), _("\nACPI Temp:"));
++        if (size < 0) {
++            acpitemp[0] = '\0';
++        } else if (static_cast<size_t>(size) >= sizeof(acpitemp)) {
++            acpitemp[sizeof(acpitemp) - 1] =  '\0';
++        } else {
++            int headlen = strlen(acpitemp);
++            getAcpiTemp(acpitemp + headlen, (sizeof(acpitemp) - headlen) / sizeof(char));
++        }
+     }
+     if (ShowCpuFreq) {
+-        sprintf(cpufreq, _("\nCPU Freq: %.3fGHz"), getCpuFreq(0) / 1e6);
+-    }
+-    char *loadmsg = cstrJoin(load, ram, swap, acpitemp, cpufreq, NULL);
++        const char CPUFREQ_FORMAT[] = "\nCPU Freq: %.3fGHz";
++        float freq = getCpuFreq(0) / 1e6;
++        size = snprintf(NULL, 0, _(CPUFREQ_FORMAT), freq);
++        if (size > 0)
++            cpufreq = new char[size + 1];
++        if (cpufreq)
++            sprintf(cpufreq, _(CPUFREQ_FORMAT), freq);
++    }
++    char *loadmsg = cstrJoin(load ? load : "",
++                             ram ? ram : "",
++                             swap ? swap : "",
++                             acpitemp,
++                             cpufreq ? cpufreq : "", NULL);
+ 
+     setToolTip(ustring(loadmsg));
++    delete [] loadmsg;
++
++    delete [] load;
++    delete [] ram;
++    delete [] swap;
++    delete [] cpufreq;
+ #elif defined HAVE_GETLOADAVG2
+     char load[31]; // enough for "CPU Load: 999.99 999.99 999.99\0"
+     double loadavg[3];
+@@ -282,7 +323,7 @@
+ 
+ int CPUStatus::getAcpiTemp(char *tempbuf, int buflen) {
+     int retbuflen = 0;
+-    char namebuf[64];
++    char namebuf[PATH_MAX];
+     char buf[64];
+ 
+     memset(tempbuf, 0, buflen);
+@@ -291,25 +332,28 @@
+         struct dirent *de;
+ 
+         while ((de = readdir(dir)) != NULL) {
+-            int fd, seglen;
++            int fd;
+ 
+             if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0)
+                 continue;
+  
+             sprintf(namebuf, "/proc/acpi/thermal_zone/%s/temperature", de->d_name);
+             fd = open(namebuf, O_RDONLY);
+-            if (fd != -1) {
++            if (fd >= 0) {
+                 int len = read(fd, buf, sizeof(buf) - 1);
+-                buf[len - 1] = '\0';
+-                seglen = strlen(buf + len - 7);
+-                if (retbuflen + seglen >= buflen) {
+-                    retbuflen = -retbuflen;
+-                    close(fd);
+-                    closedir(dir);
+-                    break;
++                if (len >= 7) {
++                    int seglen;
++                    buf[len - 1] = '\0';
++                    seglen = strlen(buf + len - 7);
++                    if (retbuflen + seglen >= buflen) {
++                        retbuflen = -retbuflen;
++                        close(fd);
++                        closedir(dir);
++                        break;
++                    }
++                    retbuflen += seglen;
++                    strncat(tempbuf, buf + len - 7, seglen);
+                 }
+-                retbuflen += seglen;
+-                strncat(tempbuf, buf + len - 7, seglen);
+                 close(fd);
+             }
+         }
+@@ -319,17 +363,28 @@
+ }
+ 
+ float CPUStatus::getCpuFreq(unsigned int cpu) {
+-    char buf[16], namebuf[64];
+-    int fd;
++    const char CPUFREQ_PATH_FORMAT[] =
++        "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_cur_freq";
++    char *namebuf = NULL;
++    int namebuf_size;
+     float cpufreq = 0;
+ 
+-    sprintf(namebuf, "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_cur_freq", cpu);
+-    fd = open(namebuf, O_RDONLY);
+-    if (fd != -1) {
+-        int len = read(fd, buf, sizeof(buf) - 1);
+-        buf[len-1] = '\0';
+-        sscanf(buf, "%f", &cpufreq);
+-        close(fd);
++    namebuf_size = snprintf(NULL, 0, CPUFREQ_PATH_FORMAT, cpu);
++    if (namebuf_size > 0)
++        namebuf = new char[namebuf_size + 1];
++    if (namebuf) {
++        sprintf(namebuf, CPUFREQ_PATH_FORMAT, cpu);
++        int fd = open(namebuf, O_RDONLY);
++        if (fd >= 0) {
++            char buf[16];
++            int len = read(fd, buf, sizeof(buf) - 1);
++            if (len > 0) {
++                buf[len-1] = '\0';
++                sscanf(buf, "%f", &cpufreq);
++            }
++            close(fd);
++        }
++        delete [] namebuf;
+     }
+     return(cpufreq);
+ }

=== modified file 'src/acpustatus.cc'
--- src/acpustatus.cc	2010-01-04 21:43:29 +0000
+++ src/acpustatus.cc	2013-10-22 21:19:12 +0000
@@ -213,38 +213,79 @@
 
 void CPUStatus::updateToolTip() {
 #ifdef linux
-    char load[31], ram[31] = "", swap[31] = "", acpitemp[61] = "", cpufreq[31] = "";
+    const char LOAD_FORMAT[] = "CPU Load: %3.2f %3.2f %3.2f, %d";
+    const float FLOAT_65K = 65536.0;
+    const float FLOAT_1M = 1048576.0f;
+    char *load = NULL, *ram = NULL, *swap = NULL, *cpufreq = NULL;
+    char acpitemp[61];
     struct sysinfo sys;
     double l1, l5, l15;
+    int size;
 
     sysinfo(&sys);
-    l1 = (float)sys.loads[0] / 65536.0;
-    l5 = (float)sys.loads[1] / 65536.0;
-    l15 = (float)sys.loads[2] / 65536.0;
-
-    sprintf(load, _("CPU Load: %3.2f %3.2f %3.2f, %d"), l1, l5, l15, sys.procs);
+    l1 = (float)sys.loads[0] / FLOAT_65K;
+    l5 = (float)sys.loads[1] / FLOAT_65K;
+    l15 = (float)sys.loads[2] / FLOAT_65K;
+
+    size = snprintf(NULL, 0, _(LOAD_FORMAT), l1, l5, l15, sys.procs);
+    if (size > 0)
+        load = new char[size + 1];
+    if (load)
+        sprintf(load, _(LOAD_FORMAT), l1, l5, l15, sys.procs);
+
     if (ShowRamUsage) {
-        float tr =(float)sys.totalram * (float)sys.mem_unit / 1048576.0f;
-        float fr =(float)sys.freeram * (float)sys.mem_unit / 1048576.0f;
-        sprintf(ram, _("\nRam: %5.2f/%.2fM"), tr, fr);
+        const char RAM_FORMAT[] = "\nRam: %5.2f/%.2fM";
+        float tr =(float)sys.totalram * (float)sys.mem_unit / FLOAT_1M;
+        float fr =(float)sys.freeram * (float)sys.mem_unit / FLOAT_1M;
+        size = snprintf(NULL, 0, _(RAM_FORMAT), tr, fr);
+        if (size > 0)
+            ram = new char[size + 1];
+        if (ram)
+            sprintf(ram, _(RAM_FORMAT), tr, fr);
     }
     if (ShowSwapUsage) {
-        float ts =(float)sys.totalswap * (float)sys.mem_unit / 1048576.0f;
-        float fs =(float)sys.freeswap * (float)sys.mem_unit / 1048576.0f;
-        sprintf(swap, _("\nSwap: %.2f/%.2fM"), ts, fs);
+        const char SWAP_FORMAT[] = "\nSwap: %.2f/%.2fM";
+        float ts =(float)sys.totalswap * (float)sys.mem_unit / FLOAT_1M;
+        float fs =(float)sys.freeswap * (float)sys.mem_unit / FLOAT_1M;
+        size = snprintf(NULL, 0, _(SWAP_FORMAT), ts, fs);
+        if (size > 0)
+            swap = new char[size + 1];
+        if (swap)
+            sprintf(swap, _(SWAP_FORMAT), ts, fs);
     }
     if (ShowAcpiTemp) {
-        int headlen;
-        sprintf(acpitemp, _("\nACPI Temp:"));
-        headlen = strlen(acpitemp);
-        getAcpiTemp(acpitemp + headlen, (sizeof(acpitemp) - headlen) / sizeof(char));
+        size = snprintf(acpitemp, sizeof(acpitemp), _("\nACPI Temp:"));
+        if (size < 0) {
+            acpitemp[0] = '\0';
+        } else if (static_cast<size_t>(size) >= sizeof(acpitemp)) {
+            acpitemp[sizeof(acpitemp) - 1] =  '\0';
+        } else {
+            int headlen = strlen(acpitemp);
+            getAcpiTemp(acpitemp + headlen, (sizeof(acpitemp) - headlen) / sizeof(char));
+        }
     }
     if (ShowCpuFreq) {
-        sprintf(cpufreq, _("\nCPU Freq: %.3fGHz"), getCpuFreq(0) / 1e6);
+        const char CPUFREQ_FORMAT[] = "\nCPU Freq: %.3fGHz";
+        float freq = getCpuFreq(0) / 1e6;
+        size = snprintf(NULL, 0, _(CPUFREQ_FORMAT), freq);
+        if (size > 0)
+            cpufreq = new char[size + 1];
+        if (cpufreq)
+            sprintf(cpufreq, _(CPUFREQ_FORMAT), freq);
     }
-    char *loadmsg = cstrJoin(load, ram, swap, acpitemp, cpufreq, NULL);
+    char *loadmsg = cstrJoin(load ? load : "",
+                             ram ? ram : "",
+                             swap ? swap : "",
+                             acpitemp,
+                             cpufreq ? cpufreq : "", NULL);
 
     setToolTip(ustring(loadmsg));
+    delete [] loadmsg;
+
+    delete [] load;
+    delete [] ram;
+    delete [] swap;
+    delete [] cpufreq;
 #elif defined HAVE_GETLOADAVG2
     char load[31]; // enough for "CPU Load: 999.99 999.99 999.99\0"
     double loadavg[3];
@@ -282,7 +323,7 @@
 
 int CPUStatus::getAcpiTemp(char *tempbuf, int buflen) {
     int retbuflen = 0;
-    char namebuf[64];
+    char namebuf[PATH_MAX];
     char buf[64];
 
     memset(tempbuf, 0, buflen);
@@ -291,25 +332,28 @@
         struct dirent *de;
 
         while ((de = readdir(dir)) != NULL) {
-            int fd, seglen;
+            int fd;
 
             if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0)
                 continue;
  
             sprintf(namebuf, "/proc/acpi/thermal_zone/%s/temperature", de->d_name);
             fd = open(namebuf, O_RDONLY);
-            if (fd != -1) {
+            if (fd >= 0) {
                 int len = read(fd, buf, sizeof(buf) - 1);
-                buf[len - 1] = '\0';
-                seglen = strlen(buf + len - 7);
-                if (retbuflen + seglen >= buflen) {
-                    retbuflen = -retbuflen;
-                    close(fd);
-                    closedir(dir);
-                    break;
+                if (len >= 7) {
+                    int seglen;
+                    buf[len - 1] = '\0';
+                    seglen = strlen(buf + len - 7);
+                    if (retbuflen + seglen >= buflen) {
+                        retbuflen = -retbuflen;
+                        close(fd);
+                        closedir(dir);
+                        break;
+                    }
+                    retbuflen += seglen;
+                    strncat(tempbuf, buf + len - 7, seglen);
                 }
-                retbuflen += seglen;
-                strncat(tempbuf, buf + len - 7, seglen);
                 close(fd);
             }
         }
@@ -319,17 +363,28 @@
 }
 
 float CPUStatus::getCpuFreq(unsigned int cpu) {
-    char buf[16], namebuf[64];
-    int fd;
+    const char CPUFREQ_PATH_FORMAT[] =
+        "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_cur_freq";
+    char *namebuf = NULL;
+    int namebuf_size;
     float cpufreq = 0;
 
-    sprintf(namebuf, "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_cur_freq", cpu);
-    fd = open(namebuf, O_RDONLY);
-    if (fd != -1) {
-        int len = read(fd, buf, sizeof(buf) - 1);
-        buf[len-1] = '\0';
-        sscanf(buf, "%f", &cpufreq);
-        close(fd);
+    namebuf_size = snprintf(NULL, 0, CPUFREQ_PATH_FORMAT, cpu);
+    if (namebuf_size > 0)
+        namebuf = new char[namebuf_size + 1];
+    if (namebuf) {
+        sprintf(namebuf, CPUFREQ_PATH_FORMAT, cpu);
+        int fd = open(namebuf, O_RDONLY);
+        if (fd >= 0) {
+            char buf[16];
+            int len = read(fd, buf, sizeof(buf) - 1);
+            if (len > 0) {
+                buf[len-1] = '\0';
+                sscanf(buf, "%f", &cpufreq);
+            }
+            close(fd);
+        }
+        delete [] namebuf;
     }
     return(cpufreq);
 }