Pedro Gimeno wrote, On 2013-12-07 16:08:
> The attached patch is not intended to be applied directly to close this
> bug; it's a "works for me" but it is likely to break packaging policies
> or builds due to unconditional use of certain C features (long long
> type, to be precise). It's also not clean in the sense that it uses "#if
> 0" ... "endif" to comment out a section of the code. I'm far behind in
> compilers, autotools, Debian build system and C dialects as to be able
> to turn it into a complete patch, but it will hopefully highlight where
> the problem lies and enable someone else to turn it into a complete
> solution.
>
> It's done against the previous version, 1.1+20120402-1. I don't think it
> will apply cleanly to 1.1+20131205 after the joining of several
> variables into a single line, but I haven't tried.
I've come up with a patch that solves all of the above:
- It applies to the latest 1.1+20131205 version and current (as of this
writing) HEAD. The former one didn't apply cleanly when I checked.
- It only makes use of features from an old standard of C, not from
newer ones like long long.
- No #if 0 commented code.
- Additionally, it adds a change notice as required by the license.
The drawback is that the limited unsigned-long-long library that this
patch adds does not inline the code, therefore it's bound to be slower.
The ASCII to integer conversion is also relatively slow compared to
atol/atoll.
The library alone is lab-tested, and I compiled wmmon with the patch and
it's working. It will take several months for me to field-test it so a
long overflows, though, assuming no downtime meanwhile.
Pedro Gimeno
diff -Nru wmmon-1.1+20131205-orig/.pc/01-cppflags.patch/wmmon/Makefile wmmon-1.1+20131205-new/.pc/01-cppflags.patch/wmmon/Makefile
--- wmmon-1.1+20131205-orig/.pc/01-cppflags.patch/wmmon/Makefile 2013-12-06 20:22:17.000000000 +0100
+++ wmmon-1.1+20131205-new/.pc/01-cppflags.patch/wmmon/Makefile 2014-03-17 17:12:37.000000000 +0100
@@ -3,6 +3,7 @@
OBJS = wmmon.o \
../wmgeneral/wmgeneral.o \
../wmgeneral/misc.o \
+ ../wmgeneral/ulllib.o \
../wmgeneral/list.o
CFLAGS = -O2
diff -Nru wmmon-1.1+20131205-orig/debian/changelog wmmon-1.1+20131205-new/debian/changelog
--- wmmon-1.1+20131205-orig/debian/changelog 2013-12-06 22:08:11.000000000 +0100
+++ wmmon-1.1+20131205-new/debian/changelog 2014-03-17 17:34:49.000000000 +0100
@@ -1,3 +1,10 @@
+wmmon (1.1+20131205-1pgimeno) unstable; urgency=low
+
+ * wmmon.c
+ - Fix overflow with longs
+
+ -- Pedro Gimeno <pgim...@email.fake> Wed, 17 Mar 2014 17:34:00 +0100
+
wmmon (1.1+20131205-1) unstable; urgency=low
* New upstream version.
diff -Nru wmmon-1.1+20131205-orig/wmgeneral/ulllib.c wmmon-1.1+20131205-new/wmgeneral/ulllib.c
--- wmmon-1.1+20131205-orig/wmgeneral/ulllib.c 1970-01-01 01:00:00.000000000 +0100
+++ wmmon-1.1+20131205-new/wmgeneral/ulllib.c 2014-03-18 18:48:28.000000000 +0100
@@ -0,0 +1,61 @@
+/*
+ * Unsigned long long arithmetic limited library, tailored for wmmon's
+ * specific needs.
+ *
+ * Copyright (c) 2014 Pedro Gimeno Fortea
+ *
+ * This file is part of wmmon.
+ *
+ * wmmon is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * wmmon is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with wmmon; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "ulllib.h"
+
+void ullreset(ullong *target) {
+ target->H = target->L = 0;
+}
+
+void ulladd(ullong *target, const ullong *toadd) {
+ unsigned long tmpL = toadd->L, tmpH = toadd->H;
+ target->L += tmpL;
+
+ /* Carry if the result is less than one of the operands */
+ target->H += tmpH + (target->L < tmpL);
+}
+
+long ullsub(const ullong *a, const ullong *b) {
+ /* Will wrap around correctly if necessary. Result is assumed to
+ fit a signed long. */
+ /* assert((a->H == b->H && a->L >= b->L)
+ || (a->H == b->H + 1 && a->L < b->L)); */
+ return a->L - b->L;
+}
+
+void ullparse(ullong *target, const char *str) {
+ ullong tmp;
+
+ ullreset(target);
+ while (*str >= '0' && *str <= '9') {
+ tmp = *target;
+ ulladd(target, target); /* *2 */
+ ulladd(target, target); /* *4 */
+ ulladd(target, &tmp); /* *5 */
+ ulladd(target, target); /* *10 */
+ tmp.H = 0;
+ tmp.L = *str - '0';
+ ulladd(target, &tmp); /* + digit */
+ ++str;
+ }
+}
diff -Nru wmmon-1.1+20131205-orig/wmgeneral/ulllib.h wmmon-1.1+20131205-new/wmgeneral/ulllib.h
--- wmmon-1.1+20131205-orig/wmgeneral/ulllib.h 1970-01-01 01:00:00.000000000 +0100
+++ wmmon-1.1+20131205-new/wmgeneral/ulllib.h 2014-03-18 18:48:42.000000000 +0100
@@ -0,0 +1,37 @@
+/*
+ * Unsigned long long arithmetic limited library, tailored for wmmon's
+ * specific needs.
+ *
+ * Copyright (c) 2014 Pedro Gimeno Fortea
+ *
+ * This file is part of wmmon.
+ *
+ * wmmon is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * wmmon is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with wmmon; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef __ULLLIB_H_
+#define __ULLLIB_H_
+
+typedef struct {
+ unsigned long H;
+ unsigned long L;
+} ullong;
+
+void ullreset(ullong *);
+void ulladd(ullong *, const ullong *);
+long ullsub(const ullong *, const ullong *);
+void ullparse(ullong *, const char *);
+
+#endif /* __LLIB_H__ */
Binary files wmmon-1.1+20131205-orig/wmgeneral/ulllib.o and wmmon-1.1+20131205-new/wmgeneral/ulllib.o differ
diff -Nru wmmon-1.1+20131205-orig/wmmon/Makefile wmmon-1.1+20131205-new/wmmon/Makefile
--- wmmon-1.1+20131205-orig/wmmon/Makefile 2014-03-17 17:11:42.000000000 +0100
+++ wmmon-1.1+20131205-new/wmmon/Makefile 2014-03-17 17:36:07.000000000 +0100
@@ -3,6 +3,7 @@
OBJS = wmmon.o \
../wmgeneral/wmgeneral.o \
../wmgeneral/misc.o \
+ ../wmgeneral/ulllib.o \
../wmgeneral/list.o
CFLAGS = -O2
diff -Nru wmmon-1.1+20131205-orig/wmmon/wmmon.c wmmon-1.1+20131205-new/wmmon/wmmon.c
--- wmmon-1.1+20131205-orig/wmmon/wmmon.c 2013-12-06 20:22:17.000000000 +0100
+++ wmmon-1.1+20131205-new/wmmon/wmmon.c 2014-03-18 18:49:24.000000000 +0100
@@ -28,6 +28,8 @@
Changes:
----
+ 17/03/2014 (Pedro Gimeno Fortea)
+ * Fix jiffy counter overflowing long on 32-bit systems.
17/06/2012 (Rodolfo GarcÃa Peñas (kix), <k...@kix.es>)
* Code style.
13/3/2012 (Barry Kelly (wbk), <coy...@devio.us>)
@@ -110,6 +112,7 @@
#include "../wmgeneral/wmgeneral.h"
#include "../wmgeneral/misc.h"
+#include "../wmgeneral/ulllib.h"
#include "wmmon-master.xpm"
#include "wmmon-mask.xbm"
@@ -202,14 +205,14 @@
int his[HISTORY_ENTRIES];
int hisaddcnt;
long rt_stat;
- long statlast;
+ ullong statlast;
long rt_idle;
- long idlelast;
+ ullong idlelast;
/* Processors stats */
long *cpu_stat;
- long *cpu_last;
+ ullong *cpu_last;
long *idle_stat;
- long *idle_last;
+ ullong *idle_last;
} stat_dev;
stat_dev stat_device[MAX_STAT_DEVICES];
@@ -220,10 +223,10 @@
int getNbCPU(void);
unsigned long getWidth(long, long);
int checksysdevs(void);
-void get_statistics(char *, long *, long *, long *, long *, long *);
+void get_statistics(char *, long *, ullong *, ullong *, ullong *, ullong *);
void DrawActive(char *);
-void update_stat_cpu(stat_dev *, long *, long *);
+void update_stat_cpu(stat_dev *, ullong *, ullong *);
void update_stat_io(stat_dev *);
void update_stat_mem(stat_dev *st, stat_dev *st2);
void update_stat_swp(stat_dev *);
@@ -245,7 +248,7 @@
int stat_online;
long starttime, curtime, nexttime;
- long istat, idle, *istat2, *idle2;
+ ullong istat, idle, *istat2, *idle2;
FILE *fp;
char *conffile = NULL;
@@ -312,9 +315,9 @@
nb_cpu = getNbCPU();
stat_device[0].cpu_stat = calloc(nb_cpu, sizeof(long));
- stat_device[0].cpu_last = calloc(nb_cpu, sizeof(long));
+ stat_device[0].cpu_last = calloc(nb_cpu, sizeof(ullong));
stat_device[0].idle_stat = calloc(nb_cpu, sizeof(long));
- stat_device[0].idle_last = calloc(nb_cpu, sizeof(long));
+ stat_device[0].idle_last = calloc(nb_cpu, sizeof(ullong));
if (!stat_device[0].cpu_stat ||
!stat_device[0].cpu_last ||
!stat_device[0].idle_stat ||
@@ -323,8 +326,8 @@
exit(1);
}
- istat2 = calloc(nb_cpu, sizeof(long));
- idle2 = calloc(nb_cpu, sizeof(long));
+ istat2 = calloc(nb_cpu, sizeof(ullong));
+ idle2 = calloc(nb_cpu, sizeof(ullong));
if (!istat2 || !idle2) {
fprintf(stderr, "%s: Unable to alloc memory !!\n", argv[0]);
exit(1);
@@ -575,16 +578,17 @@
}
-void update_stat_cpu(stat_dev *st, long *istat2, long *idle2)
+void update_stat_cpu(stat_dev *st, ullong *istat2, ullong *idle2)
{
- long k, istat, idle;
+ long k;
+ ullong istat, idle;
get_statistics(st->name, &k, &istat, &idle, istat2, idle2);
- st->rt_idle = idle - st->idlelast;
+ st->rt_idle = ullsub(&idle, &st->idlelast);
st->idlelast = idle;
- st->rt_stat = istat - st->statlast;
+ st->rt_stat = ullsub(&istat, &st->statlast);
st->statlast = istat;
if (nb_cpu > 1) {
@@ -592,10 +596,10 @@
unsigned long max, j;
cpu_max = 0; max = 0;
for (cpu = 0; cpu < nb_cpu; cpu++) {
- st->idle_stat[cpu] = idle2[cpu] - st->idle_last[cpu];
+ st->idle_stat[cpu] = ullsub(&idle2[cpu], &st->idle_last[cpu]);
st->idle_last[cpu] = idle2[cpu];
- st->cpu_stat[cpu] = istat2[cpu] - st->cpu_last[cpu];
+ st->cpu_stat[cpu] = ullsub(&istat2[cpu], &st->cpu_last[cpu]);
st->cpu_last[cpu] = istat2[cpu];
j = st->cpu_stat[cpu] + st->idle_stat[cpu];
@@ -617,7 +621,8 @@
void update_stat_io(stat_dev *st)
{
- long j, k, istat, idle;
+ long j, k;
+ ullong istat, idle;
/* Periodically re-sample. Sometimes we get anomalously high readings;
* this discards them. */
@@ -630,10 +635,10 @@
get_statistics(st->name, &k, &istat, &idle, NULL, NULL);
- st->rt_idle = idle - st->idlelast;
+ st->rt_idle = ullsub(&idle, &st->idlelast);
st->idlelast = idle;
- st->rt_stat = istat - st->statlast;
+ st->rt_stat = ullsub(&istat, &st->statlast);
st->statlast = istat;
/* remember peak for scaling of upper-right meter. */
@@ -732,7 +737,7 @@
/*******************************************************************************\
|* get_statistics *|
\*******************************************************************************/
-void get_statistics(char *devname, long *is, long *ds, long *idle, long *ds2, long *idle2)
+void get_statistics(char *devname, long *is, ullong *ds, ullong *idle, ullong *ds2, ullong *idle2)
{
int i;
static char *line = NULL;
@@ -740,10 +745,11 @@
char *p;
char *tokens = " \t\n";
float f;
+ ullong ulltmp;
*is = 0;
- *ds = 0;
- *idle = 0;
+ ullreset(ds);
+ ullreset(idle);
if (!strncmp(devname, "cpu", 3)) {
fseek(fp_stat, 0, SEEK_SET);
@@ -752,23 +758,24 @@
int cpu = -1; /* by default, cumul stats => average */
if (!strstr(line, "cpu ")) {
sscanf(line, "cpu%d", &cpu);
- ds2[cpu] = 0;
- idle2[cpu] = 0;
+ ullreset(&ds2[cpu]);
+ ullreset(&idle2[cpu]);
}
p = strtok(line, tokens);
/* 1..3, 4 == idle, we don't want idle! */
for (i=0; i<3; i++) {
p = strtok(NULL, tokens);
+ ullparse(&ulltmp, p);
if (cpu == -1)
- *ds += atol(p);
+ ulladd(ds, &ulltmp);
else
- ds2[cpu] += atol(p);
+ ulladd(&ds2[cpu], &ulltmp);
}
p = strtok(NULL, tokens);
if (cpu == -1)
- *idle = atol(p);
+ ullparse(idle, p);
else
- idle2[cpu] = atol(p);
+ ullparse(&idle2[cpu], p);
}
}
if ((fp_loadavg = freopen("/proc/loadavg", "r", fp_loadavg)) == NULL)
@@ -805,11 +812,13 @@
for (i = 1; i <= 6; i++)
p = strtok(NULL, tokens);
- *ds += atol(p);
+ ullparse(&ulltmp, p);
+ ulladd(ds, &ulltmp);
for (i = 7; i <= 10; i++)
p = strtok(NULL, tokens);
- *ds += atol(p);
+ ullparse(&ulltmp, p);
+ ulladd(ds, &ulltmp);
}
}
}
