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

Subscribers

People subscribed via source and target branches

to all changes: