Hi,

what do you think about this patch for wmmon? The question is about the "long 
long" and the cc/gcc compiler change.

Cheers,
kix
-- 
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
--- Begin Message ---
Rodolfo García Peñas wrote, On 2013-12-06 22:34:
> Hi Pedro,
> 
> I was checking the wmmon code, and I didn't find the problem. I think wmmon 
> reads the /proc/stat file using long, not using floats.
> 
> Could you check it again?
> 

Hello, Rodolfo,

I'm sorry. I use three monitoring utilities at the same time, and all
three needed patching because they were affected by the same problem,
namely that the numbers in the jiffy counters can grow beyond what these
tools are prepared to handle. The utilities are wmmon, wmcpu, and wmtop.
I just confused wmmon with wmcpu and sent it to the wrong package. My
apologies. However, this bug can still be usable for the part that
affects wmmon.

The problem in this case only affects long running machines using 32-bit
longs (i386 in particular), and happens when the jiffy counter needs
more than 32 bits to be represented (that's why they have to be
long-running).

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.

Thanks for looking into this. I will submit a separate report to wmcpu
with the previous patch I sent.

Pedro Gimeno
diff -ru wmmon-1.1+20120402/debian/changelog wmmon-1.1+20120402-new/debian/changelog
--- wmmon-1.1+20120402/debian/changelog	2012-04-18 21:09:11.000000000 +0200
+++ wmmon-1.1+20120402-new/debian/changelog	2013-05-21 16:21:52.000000000 +0200
@@ -1,3 +1,10 @@
+wmmon (1.1+20120402-1pgimeno) unstable; urgency=low
+
+  * wmmon.c
+    - Fix overflow with longs
+
+ -- Pedro Gimeno <pgim...@email.fake>  Wed, 23 Apr 2012 14:11:00 +0100
+
 wmmon (1.1+20120402-1) unstable; urgency=low
 
   * New upstream version.
diff -ru wmmon-1.1+20120402/wmmon/Makefile wmmon-1.1+20120402-new/wmmon/Makefile
--- wmmon-1.1+20120402/wmmon/Makefile	2012-04-16 12:47:01.000000000 +0200
+++ wmmon-1.1+20120402-new/wmmon/Makefile	2013-05-21 16:20:57.000000000 +0200
@@ -6,7 +6,7 @@
 		../wmgeneral/list.o
 
 CFLAGS = -O2
-CC = cc
+CC = gcc
 
 
 .c.o:
diff -ru wmmon-1.1+20120402/wmmon/wmmon.c wmmon-1.1+20120402-new/wmmon/wmmon.c
--- wmmon-1.1+20120402/wmmon/wmmon.c	2012-04-16 12:47:01.000000000 +0200
+++ wmmon-1.1+20120402-new/wmmon/wmmon.c	2013-05-21 22:57:22.000000000 +0200
@@ -130,6 +130,8 @@
 
 #define HISTORY_ENTRIES 55
 
+typedef long long llong;
+
   /********************/
  /* Global Variables */
 /********************/
@@ -218,14 +220,14 @@
 	int		his[HISTORY_ENTRIES];
 	int		hisaddcnt;
 	long	rt_stat;
-	long	statlast;
+	llong	statlast;
 	long	rt_idle;
-	long	idlelast;
+	llong	idlelast;
 	/* Processors stats */
 	long	*cpu_stat;
-	long	*cpu_last;
+	llong	*cpu_last;
 	long	*idle_stat;
-	long	*idle_last;
+	llong	*idle_last;
 	
 } stat_dev;
 
@@ -241,10 +243,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 *, llong *, llong *, llong *, llong *);
 void DrawActive(char *);
 
-void update_stat_cpu(stat_dev *, long *, long *);
+void update_stat_cpu(stat_dev *, llong *, llong *);
 void update_stat_io(stat_dev *);
 void update_stat_mem(stat_dev *st, stat_dev *st2);
 void update_stat_swp(stat_dev *);
@@ -269,10 +271,10 @@
 	long		curtime;
 	long		nexttime;
 
-	long		istat;
-	long		idle;
-	long		*istat2;
-	long		*idle2;
+	llong		istat;
+	llong		idle;
+	llong		*istat2;
+	llong		*idle2;
 
 	FILE		*fp;
 	char		*conffile = NULL;
@@ -339,9 +341,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(llong));
 	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(llong));
 	if (!stat_device[0].cpu_stat 
 			|| !stat_device[0].cpu_last 
 			|| !stat_device[0].idle_stat 
@@ -349,8 +351,8 @@
 		fprintf(stderr, "%s: Unable to alloc memory !\n", argv[0]);
 		exit(1);
 	}
-	istat2 = calloc(nb_cpu, sizeof(long));
-	idle2 = calloc(nb_cpu, sizeof(long));
+	istat2 = calloc(nb_cpu, sizeof(llong));
+	idle2 = calloc(nb_cpu, sizeof(llong));
 	if (!istat2 || !idle2) {
 		fprintf(stderr, "%s: Unable to alloc memory !!\n", argv[0]);
 		exit(1);
@@ -602,8 +604,9 @@
 	}
 }
 
-void update_stat_cpu(stat_dev *st, long *istat2, long *idle2) {
-	long	k, istat, idle;
+void update_stat_cpu(stat_dev *st, llong *istat2, llong *idle2) {
+	long	k;
+	llong	istat, idle;
 
 	get_statistics(st->name, &k, &istat, &idle, istat2, idle2);
 
@@ -639,7 +642,8 @@
 
 void update_stat_io(stat_dev *st) {
 
-	long			j, k, istat, idle;
+	long			j, k;
+	llong			istat, idle;
 
 	/* Periodically re-sample. Sometimes we get anomalously high readings;
 	 * this discards them. */
@@ -763,7 +767,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, llong *ds, llong *idle, llong *ds2, llong *idle2)
 {
 	int     i;
 	static char *line = NULL;
@@ -786,6 +790,7 @@
 					ds2[cpu] = 0;
 					idle2[cpu] = 0;
 				}
+				#if 0
 				p = strtok(line, tokens);
 				/* 1..3, 4 == idle, we don't want idle! */
 				for (i=0; i<3; i++) {
@@ -800,6 +805,17 @@
 					*idle = atol(p);
 				else
 					idle2[cpu] = atol(p);
+				#else
+				llong x,y,z;
+				if (cpu == -1) {
+					sscanf(line, "cpu %lld %lld %lld %lld", &x, &y, &z, idle);
+					*ds = x + y + z;
+				} else {
+					int n;
+					sscanf(line, "cpu%d %lld %lld %lld %lld", &n, &x, &y, &z, idle2+cpu);
+					ds2[cpu] = x + y + z;
+				}
+				#endif
 			}
 		}
 		if ((fp_loadavg = freopen("/proc/loadavg", "r", fp_loadavg)) == NULL)

--- End Message ---

Reply via email to