File 0001-Replace-the-parsing-of-proc-meminfo.patch of Package xosview

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-Replace-the-parsing-of-proc-meminfo.patch of Package xosview (Revision 77ff63d2fdd41ef67f73bb0d5461bbc6)

Currently displaying revision 77ff63d2fdd41ef67f73bb0d5461bbc6 , Show latest

Based on 607d58bf3724bdacfecd9d7ff78f7f1838c0e08f Mon Sep 17 00:00:00 2001
Based on 2d0313dd47dce8cbe4f4a5c7df13e0c7c3ab578c Mon Sep 17 00:00:00 2001
From: Mark Hills <mark.hills@framestore.com>
Date: Wed, 16 Sep 2015 14:30:12 +0100
Subject: [PATCH] Replace the parsing of /proc/meminfo

We have seen multiple problems with the existing code which manifest
themselves as either sporadic or consistent displays of warnings such
as:

meter MemMeter had a negative value of xxxxxxxxxxxxxxxxx

The previous code was hard to read due to multiple levels of
indirection; firstly it puts content into struct LineInfo, then maps
these to field_[] values. It's possible the fields are used incorrectly,
and we wanted to audit the comment 'mapped comes from cached'.

Improve the code by:

* Removing the levels of indirection through the arrays of fields
  and using named variables instead

* Remove the intermediate calculations which are done using double;
  it's likely the approximation caused by floating point is the cause
  of the sporadic errors when memory is tight and some values overflow.

* Be consistent to the comment 'cached comes from mapped'; if this is
  true then 'mapped' should be excluded from the calculation of fields_[0].

* Use a simple parser using stdio; the code is already using the
  C library and it allows us to report errors on opening the file
  and my experiences in the other parts of the code showed it to be
  less hungry on resources than fstream (this code is used in a tight
  loop)

The result is a significant decrease in lines of code, increase in
readability and hopefully fixes the above bugs (or at least possible
bugs!)
---
 linux/memmeter.cc |  158 +++++++++++++++++++++++++-----------------------------
 linux/memmeter.h  |   31 ----------
 2 files changed, 75 insertions(+), 114 deletions(-)

--- linux/memmeter.cc
+++ linux/memmeter.cc	2015-09-17 06:54:33.513518964 +0000
@@ -1,25 +1,22 @@
 //
 //  Copyright (c) 1994, 1995, 2006 by Mike Romberg ( mike.romberg@noaa.gov )
+//  Copyright (c) 2015 Framestore
 //
 //  This file may be distributed under terms of the GPL
 //
 
 #include "memmeter.h"
-#include "xosview.h"
-#include <fstream>
-#include <sstream>
+#include <limits.h>
 #include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
 
-static const char MEMFILENAME[] = "/proc/meminfo";
+#define MEMFILENAME "/proc/meminfo"
+
+#define KB(x) ((double)((x) * 1024))
 
 MemMeter::MemMeter( XOSView *parent )
 : FieldMeterGraph( parent, 6, "MEM", "USED/BUFF/SLAB/MAP/CACHE/FREE" ){
-  _MIlineInfos = NULL;
-  initLineInfo();
-}
-
-MemMeter::~MemMeter( void ){
-  delete[] _MIlineInfos;
 }
 
 void MemMeter::checkResources( void ){
@@ -38,91 +35,84 @@ void MemMeter::checkResources( void ){
 }
 
 void MemMeter::checkevent( void ){
-  getmeminfo();
-  /* for debugging (see below)
-  printf("t %4.1f used %4.1f buffer %4.1f slab %4.1f map %4.1f cache %4.1f free %4.1f\n",
-         total_/1024.0/1024.0,
-         fields_[0]/1024.0/1024.0, fields_[1]/1024.0/1024.0,
-	 fields_[2]/1024.0/1024.0, fields_[3]/1024.0/1024.0,
-	 fields_[4]/1024.0/1024.0, fields_[5]/1024.0/1024.0);
-  */
+  getstats();
   drawfields();
 }
 
-// FIXME: /proc/memstat and /proc/meminfo don't seem to correspond
-// maybe it is time to fix this in the kernel and get real infos ...
-
-void MemMeter::getmeminfo( void ){
-  getmemstat(MEMFILENAME, _MIlineInfos, _numMIlineInfos);
-  fields_[4] -= fields_[3]; // mapped comes from cache
-  fields_[0] = total_ - fields_[5] - fields_[4] - fields_[3] - fields_[2] - fields_[1];
+void MemMeter::getstats() {
+  FILE *f;
 
-  if (total_)
-    setUsed (total_ - fields_[5], total_);
-}
-
-MemMeter::LineInfo *MemMeter::findLines(LineInfo *tmplate, int len,
-                                             const char *fname){
-  std::ifstream meminfo(fname);
-  if (!meminfo){
-    std::cerr << "Can not open file : " << fname << std::endl;
+  f = fopen(MEMFILENAME, "r");
+  if (!f) {
+    perror(MEMFILENAME);
     exit(1);
   }
 
-  LineInfo *rval = new LineInfo[len];
-
-  char buf[256];
-
-  // Get the info from the "standard" meminfo file.
-  int lineNum = 0;
-  int inum = 0;  // which info are we going to insert
-  while (!meminfo.eof()){
-    meminfo.getline(buf, 256);
-    lineNum++;
-
-    for (int i = 0 ; i < len ; i++)
-      if(!strncmp(tmplate[i].id(), buf, tmplate[i].idlen())){
-        rval[inum] = tmplate[i];
-        rval[inum].line(lineNum);
-        inum++;
-      }
+  long mem_total = 0,
+    mem_free = 0,
+    buffers = 0,
+    slab = 0,
+    mapped = 0,
+    cached = 0;
+
+  for (;;) {
+    char line[128];
+    char *c, *endptr;
+    long unsigned kb = 0;
+
+    /*
+     * Parse lines in the format: "FieldName:      12345678 kB"
+     *
+     * We prefer to not use scanf because it's harder with variable
+     * number of fields; the 'kB' is not present if value is 0
+     */
+
+    if (!fgets(line, sizeof line, f))
+      break;
+
+    c = strchr(line, ':');
+    if (!c) {
+      fprintf(stderr, MEMFILENAME ": parse error, ':' expected at '%s'\n", line);
+      exit(1);
+    }
+
+    *c = '\0';
+    c++;
+
+    kb = strtoul(c, &endptr, 10);
+    if (kb == ULONG_MAX) {
+      fprintf(stderr, MEMFILENAME ": parse error, '%s' is out of range\n", c);
+      exit(1);
+    }
+
+    if (strcmp(line, "MemTotal") == 0)
+      mem_total = kb;
+    else if (strcmp(line, "MemFree") == 0)
+      mem_free = kb;
+    else if (strcmp(line, "Buffers") == 0)
+      buffers = kb;
+    else if (strcmp(line, "Cached") == 0)
+      cached = kb;
+    else if (strcmp(line, "Slab") == 0)
+      slab = kb;
+    else if (strcmp(line, "Mapped") == 0)
+      mapped = kb;
   }
 
-  return rval;
-}
+  if (fclose(f) != 0)
+    abort();
 
-void MemMeter::initLineInfo(void){
-  static LineInfo infos[] = {
-    LineInfo("MemTotal", &total_),
-    LineInfo("MemFree", &fields_[5]),
-    LineInfo("Buffers", &fields_[1]),
-    LineInfo("Slab", &fields_[2]),
-    LineInfo("Mapped", &fields_[3]),
-    LineInfo("Cached", &fields_[4])
-  };
-  _numMIlineInfos = sizeof(infos) / sizeof(LineInfo);
+  /* Don't do arithmetic on the fields_ themselves; these are floating
+   * point and when memory is large are affected by inaccuracy */
 
-  _MIlineInfos = findLines(infos, _numMIlineInfos, MEMFILENAME);
-}
+  fields_[1] = KB(buffers);
+  fields_[2] = KB(slab);
+  fields_[3] = KB(mapped);
+  fields_[4] = KB(cached);
+  fields_[5] = KB(mem_free);
 
-void MemMeter::getmemstat(const char *fname, LineInfo *infos, int ninfos){
-  std::ifstream meminfo(fname);
-  if (!meminfo){
-    std::cerr << "Can not open file : " << fname << std::endl;
-    exit(1);
-  }
+  fields_[0] = KB(mem_total - mem_free - buffers - mapped - cached - slab);
+  total_ =     KB(mem_total);
 
-  // Get the info from the "standard" meminfo file.
-  int lineNum = 0, inum = 0;
-  unsigned long long val;
-  char buf[256];
-  while (inum < ninfos && !meminfo.eof()){
-    meminfo.getline(buf, 256);
-    if (++lineNum != infos[inum].line())
-      continue;
-
-    val = strtoull(buf + infos[inum].idlen() + 1, NULL, 10);
-    /*  All stats are in KB.  */
-    infos[inum++].setVal((double)(val<<10));	/*  Multiply by 1024 bytes per K  */
-  }
+  setUsed(KB(mem_total - mem_free), KB(mem_total));
 }
--- linux/memmeter.h
+++ linux/memmeter.h	2015-09-17 06:52:05.829518344 +0000
@@ -12,42 +12,13 @@
 class MemMeter : public FieldMeterGraph {
 public:
   MemMeter( XOSView *parent );
-  ~MemMeter( void );
 
   const char *name( void ) const { return "MemMeter"; }
   void checkevent( void );
-
   void checkResources( void );
-protected:
 
-  void getmeminfo( void );
 private:
-  class LineInfo {
-  public:
-    LineInfo(const char *id, double *val)
-      { _line = -1; _id = id; _val = val; _idlen = strlen(_id); }
-    LineInfo(void) {};
-
-    int line(void) { return _line; }
-    void line(int l) { _line = l; }
-    const char *id(void) { return _id; }
-    int idlen(void) { return _idlen; }
-
-    void setVal(double val) { *_val = val; }
-
-  private:
-    int _line;
-    const char *_id;
-    int _idlen;
-    double *_val;
-  };
-
-  LineInfo *_MIlineInfos;
-  int _numMIlineInfos;
-
-  void initLineInfo(void);
-  LineInfo *findLines(LineInfo *tmplate, int len, const char *fname);
-  void getmemstat(const char *fname, LineInfo *infos, int ninfos);
+  void getstats( void );
 };

Places

File 0001-Replace-the-parsing-of-proc-meminfo.patch of Package xosview (Revision 77ff63d2fdd41ef67f73bb0d5461bbc6)

Places