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

Proposed by Chad Miller on 2013-10-22
Status: Work in progress
Proposed branch: lp:~cmiller/ubuntu/trusty/icewm/translations-cause-crash-lp447883
Merge into: lp:ubuntu/trusty/icewm
Diff against target: 400 lines (+285/-39)
4 files modified
.pc/applied-patches (+1/-0)
debian/patches/series (+1/-0)
debian/patches/tooltip-crash.patch (+189/-0)
src/acpustatus.cc (+94/-39)
To merge this branch: bzr merge lp:~cmiller/ubuntu/trusty/icewm/translations-cause-crash-lp447883
Reviewer Review Type Date Requested Status
Dmitry Shachnev Needs Fixing on 2013-11-16
Ubuntu branches 2013-10-22 Pending
Review via email: mp+192245@code.launchpad.net
To post a comment you must log in.
Dmitry Shachnev (mitya57) wrote :

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)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
14ftbfs-gcc-4.7.diff14ftbfs-gcc-4.7.diff
15icon_property_fix15icon_property_fix
16safe_get_argument16safe_get_argument
17tooltip-crash.patch
1718
=== 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 @@
15ftbfs-gcc-4.7.diff15ftbfs-gcc-4.7.diff
16icon_property_fix16icon_property_fix
17safe_get_argument17safe_get_argument
18tooltip-crash.patch
1819
=== 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 @@
1Index: icewm/src/acpustatus.cc
2===================================================================
3--- icewm.orig/src/acpustatus.cc 2013-10-22 16:48:27.202406021 -0400
4+++ icewm/src/acpustatus.cc 2013-10-22 16:56:24.396772296 -0400
5@@ -213,38 +213,79 @@
6
7 void CPUStatus::updateToolTip() {
8 #ifdef linux
9- char load[31], ram[31] = "", swap[31] = "", acpitemp[61] = "", cpufreq[31] = "";
10+ const char LOAD_FORMAT[] = "CPU Load: %3.2f %3.2f %3.2f, %d";
11+ const float FLOAT_65K = 65536.0;
12+ const float FLOAT_1M = 1048576.0f;
13+ char *load = NULL, *ram = NULL, *swap = NULL, *cpufreq = NULL;
14+ char acpitemp[61];
15 struct sysinfo sys;
16 double l1, l5, l15;
17+ int size;
18
19 sysinfo(&sys);
20- l1 = (float)sys.loads[0] / 65536.0;
21- l5 = (float)sys.loads[1] / 65536.0;
22- l15 = (float)sys.loads[2] / 65536.0;
23+ l1 = (float)sys.loads[0] / FLOAT_65K;
24+ l5 = (float)sys.loads[1] / FLOAT_65K;
25+ l15 = (float)sys.loads[2] / FLOAT_65K;
26+
27+ size = snprintf(NULL, 0, _(LOAD_FORMAT), l1, l5, l15, sys.procs);
28+ if (size > 0)
29+ load = new char[size + 1];
30+ if (load)
31+ sprintf(load, _(LOAD_FORMAT), l1, l5, l15, sys.procs);
32
33- sprintf(load, _("CPU Load: %3.2f %3.2f %3.2f, %d"), l1, l5, l15, sys.procs);
34 if (ShowRamUsage) {
35- float tr =(float)sys.totalram * (float)sys.mem_unit / 1048576.0f;
36- float fr =(float)sys.freeram * (float)sys.mem_unit / 1048576.0f;
37- sprintf(ram, _("\nRam: %5.2f/%.2fM"), tr, fr);
38+ const char RAM_FORMAT[] = "\nRam: %5.2f/%.2fM";
39+ float tr =(float)sys.totalram * (float)sys.mem_unit / FLOAT_1M;
40+ float fr =(float)sys.freeram * (float)sys.mem_unit / FLOAT_1M;
41+ size = snprintf(NULL, 0, _(RAM_FORMAT), tr, fr);
42+ if (size > 0)
43+ ram = new char[size + 1];
44+ if (ram)
45+ sprintf(ram, _(RAM_FORMAT), tr, fr);
46 }
47 if (ShowSwapUsage) {
48- float ts =(float)sys.totalswap * (float)sys.mem_unit / 1048576.0f;
49- float fs =(float)sys.freeswap * (float)sys.mem_unit / 1048576.0f;
50- sprintf(swap, _("\nSwap: %.2f/%.2fM"), ts, fs);
51+ const char SWAP_FORMAT[] = "\nSwap: %.2f/%.2fM";
52+ float ts =(float)sys.totalswap * (float)sys.mem_unit / FLOAT_1M;
53+ float fs =(float)sys.freeswap * (float)sys.mem_unit / FLOAT_1M;
54+ size = snprintf(NULL, 0, _(SWAP_FORMAT), ts, fs);
55+ if (size > 0)
56+ swap = new char[size + 1];
57+ if (swap)
58+ sprintf(swap, _(SWAP_FORMAT), ts, fs);
59 }
60 if (ShowAcpiTemp) {
61- int headlen;
62- sprintf(acpitemp, _("\nACPI Temp:"));
63- headlen = strlen(acpitemp);
64- getAcpiTemp(acpitemp + headlen, (sizeof(acpitemp) - headlen) / sizeof(char));
65+ size = snprintf(acpitemp, sizeof(acpitemp), _("\nACPI Temp:"));
66+ if (size < 0) {
67+ acpitemp[0] = '\0';
68+ } else if (static_cast<size_t>(size) >= sizeof(acpitemp)) {
69+ acpitemp[sizeof(acpitemp) - 1] = '\0';
70+ } else {
71+ int headlen = strlen(acpitemp);
72+ getAcpiTemp(acpitemp + headlen, (sizeof(acpitemp) - headlen) / sizeof(char));
73+ }
74 }
75 if (ShowCpuFreq) {
76- sprintf(cpufreq, _("\nCPU Freq: %.3fGHz"), getCpuFreq(0) / 1e6);
77- }
78- char *loadmsg = cstrJoin(load, ram, swap, acpitemp, cpufreq, NULL);
79+ const char CPUFREQ_FORMAT[] = "\nCPU Freq: %.3fGHz";
80+ float freq = getCpuFreq(0) / 1e6;
81+ size = snprintf(NULL, 0, _(CPUFREQ_FORMAT), freq);
82+ if (size > 0)
83+ cpufreq = new char[size + 1];
84+ if (cpufreq)
85+ sprintf(cpufreq, _(CPUFREQ_FORMAT), freq);
86+ }
87+ char *loadmsg = cstrJoin(load ? load : "",
88+ ram ? ram : "",
89+ swap ? swap : "",
90+ acpitemp,
91+ cpufreq ? cpufreq : "", NULL);
92
93 setToolTip(ustring(loadmsg));
94+ delete [] loadmsg;
95+
96+ delete [] load;
97+ delete [] ram;
98+ delete [] swap;
99+ delete [] cpufreq;
100 #elif defined HAVE_GETLOADAVG2
101 char load[31]; // enough for "CPU Load: 999.99 999.99 999.99\0"
102 double loadavg[3];
103@@ -282,7 +323,7 @@
104
105 int CPUStatus::getAcpiTemp(char *tempbuf, int buflen) {
106 int retbuflen = 0;
107- char namebuf[64];
108+ char namebuf[PATH_MAX];
109 char buf[64];
110
111 memset(tempbuf, 0, buflen);
112@@ -291,25 +332,28 @@
113 struct dirent *de;
114
115 while ((de = readdir(dir)) != NULL) {
116- int fd, seglen;
117+ int fd;
118
119 if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0)
120 continue;
121
122 sprintf(namebuf, "/proc/acpi/thermal_zone/%s/temperature", de->d_name);
123 fd = open(namebuf, O_RDONLY);
124- if (fd != -1) {
125+ if (fd >= 0) {
126 int len = read(fd, buf, sizeof(buf) - 1);
127- buf[len - 1] = '\0';
128- seglen = strlen(buf + len - 7);
129- if (retbuflen + seglen >= buflen) {
130- retbuflen = -retbuflen;
131- close(fd);
132- closedir(dir);
133- break;
134+ if (len >= 7) {
135+ int seglen;
136+ buf[len - 1] = '\0';
137+ seglen = strlen(buf + len - 7);
138+ if (retbuflen + seglen >= buflen) {
139+ retbuflen = -retbuflen;
140+ close(fd);
141+ closedir(dir);
142+ break;
143+ }
144+ retbuflen += seglen;
145+ strncat(tempbuf, buf + len - 7, seglen);
146 }
147- retbuflen += seglen;
148- strncat(tempbuf, buf + len - 7, seglen);
149 close(fd);
150 }
151 }
152@@ -319,17 +363,28 @@
153 }
154
155 float CPUStatus::getCpuFreq(unsigned int cpu) {
156- char buf[16], namebuf[64];
157- int fd;
158+ const char CPUFREQ_PATH_FORMAT[] =
159+ "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_cur_freq";
160+ char *namebuf = NULL;
161+ int namebuf_size;
162 float cpufreq = 0;
163
164- sprintf(namebuf, "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_cur_freq", cpu);
165- fd = open(namebuf, O_RDONLY);
166- if (fd != -1) {
167- int len = read(fd, buf, sizeof(buf) - 1);
168- buf[len-1] = '\0';
169- sscanf(buf, "%f", &cpufreq);
170- close(fd);
171+ namebuf_size = snprintf(NULL, 0, CPUFREQ_PATH_FORMAT, cpu);
172+ if (namebuf_size > 0)
173+ namebuf = new char[namebuf_size + 1];
174+ if (namebuf) {
175+ sprintf(namebuf, CPUFREQ_PATH_FORMAT, cpu);
176+ int fd = open(namebuf, O_RDONLY);
177+ if (fd >= 0) {
178+ char buf[16];
179+ int len = read(fd, buf, sizeof(buf) - 1);
180+ if (len > 0) {
181+ buf[len-1] = '\0';
182+ sscanf(buf, "%f", &cpufreq);
183+ }
184+ close(fd);
185+ }
186+ delete [] namebuf;
187 }
188 return(cpufreq);
189 }
0190
=== 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 @@
213213
214void CPUStatus::updateToolTip() {214void CPUStatus::updateToolTip() {
215#ifdef linux215#ifdef linux
216 char load[31], ram[31] = "", swap[31] = "", acpitemp[61] = "", cpufreq[31] = "";216 const char LOAD_FORMAT[] = "CPU Load: %3.2f %3.2f %3.2f, %d";
217 const float FLOAT_65K = 65536.0;
218 const float FLOAT_1M = 1048576.0f;
219 char *load = NULL, *ram = NULL, *swap = NULL, *cpufreq = NULL;
220 char acpitemp[61];
217 struct sysinfo sys;221 struct sysinfo sys;
218 double l1, l5, l15;222 double l1, l5, l15;
223 int size;
219224
220 sysinfo(&sys);225 sysinfo(&sys);
221 l1 = (float)sys.loads[0] / 65536.0;226 l1 = (float)sys.loads[0] / FLOAT_65K;
222 l5 = (float)sys.loads[1] / 65536.0;227 l5 = (float)sys.loads[1] / FLOAT_65K;
223 l15 = (float)sys.loads[2] / 65536.0;228 l15 = (float)sys.loads[2] / FLOAT_65K;
224229
225 sprintf(load, _("CPU Load: %3.2f %3.2f %3.2f, %d"), l1, l5, l15, sys.procs);230 size = snprintf(NULL, 0, _(LOAD_FORMAT), l1, l5, l15, sys.procs);
231 if (size > 0)
232 load = new char[size + 1];
233 if (load)
234 sprintf(load, _(LOAD_FORMAT), l1, l5, l15, sys.procs);
235
226 if (ShowRamUsage) {236 if (ShowRamUsage) {
227 float tr =(float)sys.totalram * (float)sys.mem_unit / 1048576.0f;237 const char RAM_FORMAT[] = "\nRam: %5.2f/%.2fM";
228 float fr =(float)sys.freeram * (float)sys.mem_unit / 1048576.0f;238 float tr =(float)sys.totalram * (float)sys.mem_unit / FLOAT_1M;
229 sprintf(ram, _("\nRam: %5.2f/%.2fM"), tr, fr);239 float fr =(float)sys.freeram * (float)sys.mem_unit / FLOAT_1M;
240 size = snprintf(NULL, 0, _(RAM_FORMAT), tr, fr);
241 if (size > 0)
242 ram = new char[size + 1];
243 if (ram)
244 sprintf(ram, _(RAM_FORMAT), tr, fr);
230 }245 }
231 if (ShowSwapUsage) {246 if (ShowSwapUsage) {
232 float ts =(float)sys.totalswap * (float)sys.mem_unit / 1048576.0f;247 const char SWAP_FORMAT[] = "\nSwap: %.2f/%.2fM";
233 float fs =(float)sys.freeswap * (float)sys.mem_unit / 1048576.0f;248 float ts =(float)sys.totalswap * (float)sys.mem_unit / FLOAT_1M;
234 sprintf(swap, _("\nSwap: %.2f/%.2fM"), ts, fs);249 float fs =(float)sys.freeswap * (float)sys.mem_unit / FLOAT_1M;
250 size = snprintf(NULL, 0, _(SWAP_FORMAT), ts, fs);
251 if (size > 0)
252 swap = new char[size + 1];
253 if (swap)
254 sprintf(swap, _(SWAP_FORMAT), ts, fs);
235 }255 }
236 if (ShowAcpiTemp) {256 if (ShowAcpiTemp) {
237 int headlen;257 size = snprintf(acpitemp, sizeof(acpitemp), _("\nACPI Temp:"));
238 sprintf(acpitemp, _("\nACPI Temp:"));258 if (size < 0) {
239 headlen = strlen(acpitemp);259 acpitemp[0] = '\0';
240 getAcpiTemp(acpitemp + headlen, (sizeof(acpitemp) - headlen) / sizeof(char));260 } else if (static_cast<size_t>(size) >= sizeof(acpitemp)) {
261 acpitemp[sizeof(acpitemp) - 1] = '\0';
262 } else {
263 int headlen = strlen(acpitemp);
264 getAcpiTemp(acpitemp + headlen, (sizeof(acpitemp) - headlen) / sizeof(char));
265 }
241 }266 }
242 if (ShowCpuFreq) {267 if (ShowCpuFreq) {
243 sprintf(cpufreq, _("\nCPU Freq: %.3fGHz"), getCpuFreq(0) / 1e6);268 const char CPUFREQ_FORMAT[] = "\nCPU Freq: %.3fGHz";
269 float freq = getCpuFreq(0) / 1e6;
270 size = snprintf(NULL, 0, _(CPUFREQ_FORMAT), freq);
271 if (size > 0)
272 cpufreq = new char[size + 1];
273 if (cpufreq)
274 sprintf(cpufreq, _(CPUFREQ_FORMAT), freq);
244 }275 }
245 char *loadmsg = cstrJoin(load, ram, swap, acpitemp, cpufreq, NULL);276 char *loadmsg = cstrJoin(load ? load : "",
277 ram ? ram : "",
278 swap ? swap : "",
279 acpitemp,
280 cpufreq ? cpufreq : "", NULL);
246281
247 setToolTip(ustring(loadmsg));282 setToolTip(ustring(loadmsg));
283 delete [] loadmsg;
284
285 delete [] load;
286 delete [] ram;
287 delete [] swap;
288 delete [] cpufreq;
248#elif defined HAVE_GETLOADAVG2289#elif defined HAVE_GETLOADAVG2
249 char load[31]; // enough for "CPU Load: 999.99 999.99 999.99\0"290 char load[31]; // enough for "CPU Load: 999.99 999.99 999.99\0"
250 double loadavg[3];291 double loadavg[3];
@@ -282,7 +323,7 @@
282323
283int CPUStatus::getAcpiTemp(char *tempbuf, int buflen) {324int CPUStatus::getAcpiTemp(char *tempbuf, int buflen) {
284 int retbuflen = 0;325 int retbuflen = 0;
285 char namebuf[64];326 char namebuf[PATH_MAX];
286 char buf[64];327 char buf[64];
287328
288 memset(tempbuf, 0, buflen);329 memset(tempbuf, 0, buflen);
@@ -291,25 +332,28 @@
291 struct dirent *de;332 struct dirent *de;
292333
293 while ((de = readdir(dir)) != NULL) {334 while ((de = readdir(dir)) != NULL) {
294 int fd, seglen;335 int fd;
295336
296 if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0)337 if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0)
297 continue;338 continue;
298 339
299 sprintf(namebuf, "/proc/acpi/thermal_zone/%s/temperature", de->d_name);340 sprintf(namebuf, "/proc/acpi/thermal_zone/%s/temperature", de->d_name);
300 fd = open(namebuf, O_RDONLY);341 fd = open(namebuf, O_RDONLY);
301 if (fd != -1) {342 if (fd >= 0) {
302 int len = read(fd, buf, sizeof(buf) - 1);343 int len = read(fd, buf, sizeof(buf) - 1);
303 buf[len - 1] = '\0';344 if (len >= 7) {
304 seglen = strlen(buf + len - 7);345 int seglen;
305 if (retbuflen + seglen >= buflen) {346 buf[len - 1] = '\0';
306 retbuflen = -retbuflen;347 seglen = strlen(buf + len - 7);
307 close(fd);348 if (retbuflen + seglen >= buflen) {
308 closedir(dir);349 retbuflen = -retbuflen;
309 break;350 close(fd);
351 closedir(dir);
352 break;
353 }
354 retbuflen += seglen;
355 strncat(tempbuf, buf + len - 7, seglen);
310 }356 }
311 retbuflen += seglen;
312 strncat(tempbuf, buf + len - 7, seglen);
313 close(fd);357 close(fd);
314 }358 }
315 }359 }
@@ -319,17 +363,28 @@
319}363}
320364
321float CPUStatus::getCpuFreq(unsigned int cpu) {365float CPUStatus::getCpuFreq(unsigned int cpu) {
322 char buf[16], namebuf[64];366 const char CPUFREQ_PATH_FORMAT[] =
323 int fd;367 "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_cur_freq";
368 char *namebuf = NULL;
369 int namebuf_size;
324 float cpufreq = 0;370 float cpufreq = 0;
325371
326 sprintf(namebuf, "/sys/devices/system/cpu/cpu%d/cpufreq/scaling_cur_freq", cpu);372 namebuf_size = snprintf(NULL, 0, CPUFREQ_PATH_FORMAT, cpu);
327 fd = open(namebuf, O_RDONLY);373 if (namebuf_size > 0)
328 if (fd != -1) {374 namebuf = new char[namebuf_size + 1];
329 int len = read(fd, buf, sizeof(buf) - 1);375 if (namebuf) {
330 buf[len-1] = '\0';376 sprintf(namebuf, CPUFREQ_PATH_FORMAT, cpu);
331 sscanf(buf, "%f", &cpufreq);377 int fd = open(namebuf, O_RDONLY);
332 close(fd);378 if (fd >= 0) {
379 char buf[16];
380 int len = read(fd, buf, sizeof(buf) - 1);
381 if (len > 0) {
382 buf[len-1] = '\0';
383 sscanf(buf, "%f", &cpufreq);
384 }
385 close(fd);
386 }
387 delete [] namebuf;
333 }388 }
334 return(cpufreq);389 return(cpufreq);
335}390}

Subscribers

People subscribed via source and target branches

to all changes: