mtr v0.59

 - Josh Martin suggested to add some bounds checking to the dynamic
   field code. This caused me to delve in, and rewrite some
   things. Now 50 lines of code less, but cleaner code. :-)

source: ftp://ftp.bitwizard.nl/mtr/mtr-0.59.tar.gz

diff --git a/AUTHORS b/AUTHORS
index a7f9644eac8f5e51d6d712efdde6b7ced2d88a59..6dedd6a2c402b03547a7daa9b4a6d03614e03825 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -26,6 +26,7 @@
         Craig Milo Rogers (Rogers@ISI.EDU)
         Russell Nelson (rn-mtr@crynwr.com)
         Davin Milun (milun@acm.org)
+        Josh Martin (jmartin@columbiaservices.net)
         Alexander V. Lukyanov (lav@yars.free.net)
         Charles Levert (charles@comm.polymtl.ca) 
         Bertrand Leconte (B.Leconte@mail.dotcom.fr)

diff --git a/NEWS b/NEWS
index 18c4b87e5582a53dd99b22c85f0ece5685837814..9c9a3b4d8abefc0065720374bdfabee9aa9cc7ec 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,12 @@
 WHAT'S NEW?
+  v0.59 Josh Martin suggested to add some bounds checking to 
+        the dynamic field code. This caused me to delve in, and
+        rewrite some things. Now 50 lines of code less, but cleaner
+        code. :-)
+
+  v0.58 I don't remember. Fogot to update this. :-( Check the 
+        patch.
+
   v0.57 Lots of whitespace cleanups. And a DNS fix: Don't do DNS
         lookups in raw mode with -n specified. 
 

diff --git a/README b/README
index 385cd2e38c496360ceb25abd89ad6476f7dbb437..b35de7d72201cc2c76ec2b1cc050c684661e1a67 100644 (file)
--- a/README
+++ b/README
@@ -75,3 +75,8 @@ WHERE CAN I GET THE LATEST VERSION OR MORE INFORMATION?
   jitterbug bug-tracking system at http://www.BitWizard.nl/cgi-bin/mtr .
   If you don't have web-access, mail the mailinglist. 
 
+  Patches can be submitted by Email to me, or submitted to the 
+  bug tracking system. Please use unified diffs. 
+
+-- REW
+

diff --git a/configure.in b/configure.in
index 9a8fd087a92147924e93dcec6a3278c322cf0201..f536dabf48af07a1384e949099f5d46034d87020 100644 (file)
--- a/configure.in
+++ b/configure.in
@@ -1,5 +1,5 @@
 AC_INIT(mtr.c)
-AM_INIT_AUTOMAKE(mtr, 0.58)
+AM_INIT_AUTOMAKE(mtr, 0.59)
 
 
 AC_SUBST(GTK_OBJ)

diff --git a/curses.c b/curses.c
index 16349169ecdf847a2998f6dacb9e68d392afa452..d9ef8fd04b34728c3b87122f3bd8a11e42addb51 100644 (file)
--- a/curses.c
+++ b/curses.c
@@ -61,7 +61,6 @@
 #include <time.h>
 
 extern char LocalHostname[];
-extern char fld_active[];
 extern int fstTTL;
 extern int maxTTL;
 extern int packetsize;
@@ -70,38 +69,6 @@ extern int tos;
 extern float WaitTime;
 
 
-struct fields data_fields[MAXFLD] = {
-  /* Remark, Header, Format, Width, CallBackFunc */
-  { "<sp>: Space between fields", " ",  " ",        1, &net_drop  },   /* 0 */
-  { "L: Loss Ratio",          "Loss%",  " %4.1f%%", 6, &net_loss  },   /* 1 */
-  { "D: Dropped Packets",     "Drop",   " %4d",     5, &net_drop  },   /* 2 */
-  { "R: Received Packets",    "Rcv",    " %5d",     6, &net_returned}, /* 3 */
-  { "S: Sent Packets",        "Snt",    " %5d",     6, &net_xmit  },   /* 4 */
-  { "N: Newest RTT(ms)",      "Last",   " %5.1f",   6, &net_last  },   /* 5 */
-  { "B: Min/Best RTT(ms)",    "Best",   " %5.1f",   6, &net_best  },   /* 6 */
-  { "A: Average RTT(ms)",     "Avg",    " %5.1f",   6, &net_avg   },   /* 7 */
-  { "W: Max/Worst RTT(ms)",   "Wrst",   " %5.1f",   6, &net_worst },   /* 8 */
-  { "V: Standard Deviation",  "StDev",  " %5.1f",   6, &net_stdev },   /* 9 */
-  { "G: Geometric Mean",      "Gmean",  " %5.1f",   6, &net_gmean },   /* 10 */
-  { "J: Current Jitter",      "Jttr",   " %4.1f",   5, &net_jitter},   /* 11 */
-  { "M: Jitter Mean/Avg.",    "Javg",   " %4.1f",   5, &net_javg  },   /* 12 */
-  { "X: Worst Jitter",        "Jmax",   " %4.1f",   5, &net_jworst},   /* 13 */
-  { "I: Interarrival Jitter", "Jint",   " %4.1f",   5, &net_jinta },   /* 14 */
-  { 0, 0, 0, 0, 0 }
-};
-
-
-/* keys: the value in the array is the index number in data_fields[] */
-int fld_index[] = {
-   0, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,           /* ' ', 0,1..9 */
-   7,  6, -1,  2, -1, -1, 10, -1, 14, 11, -1,  1, 12,   /* A..M */
-   5, -1, -1, -1,  3,  4, -1, -1,  9,  8, 13, -1, -1,   /* N..Z */
-  -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,   /* a..m */
-  -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,   /* n..z */
-  -1
-};
-
-
 void pwcenter(char *str) 
 {
   int maxx, maxy;
@@ -254,10 +221,12 @@ int mtr_curses_keyaction()
     refresh();
 
     i = 0;
-    while ( (c=getch ()) != '\n' && i<MAXFLD ) {
-      attron(A_BOLD); printw("%c", c); attroff(A_BOLD); refresh();
-      if( (c>= 'A' && c<= 'Z') || c==' ') {
-        buf[i++] = c; /* only accept [ A-Z], can be extend to [a-z0-9] */
+    while ( (c=getch ()) != '\n' && i < MAXFLD ) {
+      if( strchr(available_options, c) ) {
+        attron(A_BOLD); printw("%c", c); attroff(A_BOLD); refresh();
+        buf[i++] = c; /* Only permit values in "available_options" be entered */
+      } else {
+        printf("\a"); /* Illegal character. Beep, ring the bell. */
       }
     }
     buf[i] = '\0';
@@ -334,42 +303,34 @@ void mtr_curses_hosts(int startstat)
       /* changedByMin */
       hd_len = 0;
       for( i=0; i<MAXFLD; i++ ) {
-       // assume only a-zA-Z are valid options, and error checking
-       // is done in the input side?
-       if( fld_active[i]>= 'a' && fld_active[i]<= 'z') {
-         j = fld_active[i] - 'a' + 11 + 26; 
-       } else if( fld_active[i]>= 'A' && fld_active[i]<= 'Z') {
-         j = fld_active[i] - 'A' + 11; 
-       } else if( fld_active[i]>= '0' && fld_active[i]<= '9') {
-         j = fld_active[i] - '0' +1; 
-       } else if( fld_active[i] == ' ' ) {
-         j = 0;
-       } else {
-         continue;     /* ignore stuff don't understand */
-       }
+       /* Ignore options that don't exist */
+       /* On the other hand, we now check the input side. Shouldn't happen, 
+          can't be careful enough. */
+       j = fld_index[fld_active[i]];
+       if (j == -1) continue; 
 
        /* temporay hack for stats usec to ms... */
-       if( index( data_fields[ fld_index[j] ].format, 'f' ) ) {
-         sprintf(buf + hd_len, data_fields[ fld_index[j] ].format,
-               data_fields[ fld_index[j] ].net_xxx(at) /1000.0 );
+       if( index( data_fields[j].format, 'f' ) ) {
+         sprintf(buf + hd_len, data_fields[j].format,
+               data_fields[j].net_xxx(at) /1000.0 );
        } else {
-         sprintf(buf + hd_len, data_fields[ fld_index[j] ].format,
-               data_fields[ fld_index[j] ].net_xxx(at) );
+         sprintf(buf + hd_len, data_fields[j].format,
+               data_fields[j].net_xxx(at) );
        }
-       hd_len +=  data_fields[ fld_index[j] ].length;
+       hd_len +=  data_fields[j].length;
       }
       buf[hd_len] = 0;
       printw("%s", buf);
 
       /* Multi path by Min */
-      for( i=0; i<MAXPATH; i++ ) {
+      for (i=0; i < MAXPATH; i++ ) {
         addrs = net_addrs(at, i);
-       if( addrs == addr ) continue;
-       if( addrs == 0 ) break;
+       if (addrs == addr) continue;
+       if (addrs == 0) break;
 
         name = dns_lookup(addrs);
         if (! net_up(at)) attron(A_BOLD);
-        if(name != NULL) {
+        if (name != NULL) {
          printw("\n    %s", name);
         } else {
          printw("\n    %d.%d.%d.%d",
@@ -549,24 +510,13 @@ void mtr_curses_redraw()
   
   if (display_mode == 0) {
     /* changedByMin */
-    for( i=0; i<MAXFLD; i++ ) {
-       // assume only 0-9A-Za-z are valid options, and error checking
-       // is down on the input side
-       if( fld_active[i]>= 'a' && fld_active[i]<= 'z') {
-         j = fld_active[i] - 'a' + 11 + 26; 
-       } else if( fld_active[i]>= 'A' && fld_active[i]<= 'Z') {
-         j = fld_active[i] - 'A' + 11; 
-       } else if( fld_active[i]>= '0' && fld_active[i]<= '9') {
-         j = fld_active[i] - '0' +1; 
-       } else if( fld_active[i] == ' ' ) {
-         j = 0;
-       } else {
-         continue;     /* ignore unknown */
-       }
+    for (i=0; i < MAXFLD; i++ ) {
+       j = fld_index[fld_active[i]];
+       if (j < 0) continue;
 
-       sprintf( fmt, "%%%ds", data_fields[fld_index[j]].length );
-        sprintf( buf + hd_len, fmt, data_fields[fld_index[j]].title );
-       hd_len +=  data_fields[fld_index[j]].length;
+       sprintf( fmt, "%%%ds", data_fields[j].length );
+        sprintf( buf + hd_len, fmt, data_fields[j].title );
+       hd_len +=  data_fields[j].length;
     }
     attron(A_BOLD);
     mvprintw(rowstat - 1, 0, " Host");
@@ -605,6 +555,7 @@ void mtr_curses_redraw()
   refresh();
 }
 
+
 void mtr_curses_open() 
 {
   initscr();
@@ -614,12 +565,14 @@ void mtr_curses_open()
   mtr_curses_redraw();
 }
 
+
 void mtr_curses_close() 
 {  
   printw("\n");
   endwin();
 }
 
+
 void mtr_curses_clear() 
 {
   mtr_curses_close();

diff --git a/mtr.c b/mtr.c
index 1036eb3c8f4920045f67b58173bca56ed770e707..fccfecad6a2771d6070454230d71fbfcd96deac9 100644 (file)
--- a/mtr.c
+++ b/mtr.c
@@ -75,13 +75,51 @@ int maxTTL = 30;            /* inline with traceroute */
 int af = DEFAULT_AF;
 
 /* default display field(defined by key in net.h) and order */
-char fld_active[2*MAXFLD] = "LS NABWV";
+unsigned char fld_active[2*MAXFLD] = "LS NABWV";
+char fld_index[256];
+char available_options[MAXFLD];
+
+
+struct fields data_fields[MAXFLD] = {
+  /* key, Remark, Header, Format, Width, CallBackFunc */
+  {' ', "<sp>: Space between fields", " ",  " ",        1, &net_drop  },   /* 0 */
+  {'L', "L: Loss Ratio",          "Loss%",  " %4.1f%%", 6, &net_loss  },   /* 1 */
+  {'D', "D: Dropped Packets",     "Drop",   " %4d",     5, &net_drop  },   /* 2 */
+  {'R', "R: Received Packets",    "Rcv",    " %5d",     6, &net_returned}, /* 3 */
+  {'S', "S: Sent Packets",        "Snt",    " %5d",     6, &net_xmit  },   /* 4 */
+  {'N', "N: Newest RTT(ms)",      "Last",   " %5.1f",   6, &net_last  },   /* 5 */
+  {'B', "B: Min/Best RTT(ms)",    "Best",   " %5.1f",   6, &net_best  },   /* 6 */
+  {'A', "A: Average RTT(ms)",     "Avg",    " %5.1f",   6, &net_avg   },   /* 7 */
+  {'W', "W: Max/Worst RTT(ms)",   "Wrst",   " %5.1f",   6, &net_worst },   /* 8 */
+  {'V', "V: Standard Deviation",  "StDev",  " %5.1f",   6, &net_stdev },   /* 9 */
+  {'G', "G: Geometric Mean",      "Gmean",  " %5.1f",   6, &net_gmean },   /* 10 */
+  {'J', "J: Current Jitter",      "Jttr",   " %4.1f",   5, &net_jitter},   /* 11 */
+  {'M', "M: Jitter Mean/Avg.",    "Javg",   " %4.1f",   5, &net_javg  },   /* 12 */
+  {'X', "X: Worst Jitter",        "Jmax",   " %4.1f",   5, &net_jworst},   /* 13 */
+  {'I', "I: Interarrival Jitter", "Jint",   " %4.1f",   5, &net_jinta },   /* 14 */
+  {'\0', 0, 0, 0, 0, 0 }
+};
+
+
+void init_fld_options (void)
+{
+  int i;
+
+  for (i=0;i < 256;i++)
+    fld_index[i] = -1;
 
+  for (i=0;data_fields[i].key != 0;i++) {
+    available_options[i] = data_fields[i].key;
+    fld_index[data_fields[i].key] = i;
+  }
+  available_options[i] = 0;
+}
 
 
 void parse_arg(int argc, char **argv) 
 {
   int opt;
+  int i;
   static struct option long_options[] = {
     { "version", 0, 0, 'v' },
     { "help", 0, 0, 'h' },
@@ -187,13 +225,23 @@ void parse_arg(int argc, char **argv)
       if( maxTTL < 1) {                       /* prevent 0 hop */
        maxTTL = 1;
       }
-      if( fstTTL > maxTTL ) {         /* don't know the pos of -m or -f */
+      if (fstTTL > maxTTL) {         /* don't know the pos of -m or -f */
        fstTTL = maxTTL;
       }
       break;
     case 'o':
-      /* XXX no error checking on the input string, lazy */
-      strncpy (fld_active, optarg, MAXFLD-1 );
+      /* Check option before passing it on to fld_active. */
+      if (strlen (optarg) > MAXFLD) {
+       fprintf (stderr, "Too many fields: %s\n", optarg);
+        exit (1);
+      }
+      for (i=0; optarg[i]; i++) {
+        if(!strchr(available_options, optarg[i])) {
+          fprintf (stderr, "Unknown field identifier: %c\n", optarg[i]);
+          exit (1);
+        }
+      }
+      strcpy (fld_active, optarg);
       break;
     case 'b':
       bitpattern = atoi (optarg);
@@ -282,6 +330,10 @@ int main(int argc, char **argv) {
   
   display_detect(&argc, &argv);
 
+  /* The field options are now in a static array all together, 
+     but that requires a run-time initialization. -- REW */
+  init_fld_options ();
+
   parse_mtr_options (getenv ("MTR_OPTIONS"));
 
   parse_arg(argc, argv);

diff --git a/net.h b/net.h
index 25740b9e497113c1a6b290b3c08e10d1356f6eab..f4a0060c91b475c652d2b05ecfcfc0ccb140735c 100644 (file)
--- a/net.h
+++ b/net.h
@@ -82,8 +82,12 @@ int net_duplicate(int at, int seq);
 #endif
 
 
+/* XXX This doesn't really belong in this header file, but as the
+   right c-files include it, it will have to do for now. -- REW */
+
 /* dynamic field drawing */
 struct fields {
+  CONST unsigned char key;
   CONST char *descr;
   CONST char *title;
   CONST char *format;
@@ -95,4 +99,7 @@ extern struct fields data_fields[MAXFLD];
 
 
 /* keys: the value in the array is the index number in data_fields[] */
-extern  int fld_index[];
+extern char fld_index[];
+extern unsigned char fld_active[];
+extern char available_options[];
+

diff --git a/report.c b/report.c
index b254648de91a9d52923731de212e4a2dd2283a6f..b979650607b3a2376bd4624acb69231a26c5e921 100644 (file)
--- a/report.c
+++ b/report.c
@@ -31,7 +31,6 @@
 extern int dns;
 extern char LocalHostname[];
 extern char *Hostname;
-extern char fld_active[];
 extern int fstTTL;
 extern int maxTTL;
 extern int packetsize;
@@ -52,20 +51,12 @@ void report_close() {
 
   sprintf(buf, "HOST: %-33s", LocalHostname);
   for( i=0; i<MAXFLD; i++ ) {
-    if( fld_active[i]>= 'a' && fld_active[i]<= 'z') {
-      j = fld_active[i] - 'a' + 11 + 26;
-    } else if( fld_active[i]>= 'A' && fld_active[i]<= 'Z') {
-      j = fld_active[i] - 'A' + 11;
-    } else if( fld_active[i]>= '0' && fld_active[i]<= '9') {
-      j = fld_active[i] - '0' +1;
-    } else if( fld_active[i] == ' ' ) {
-      j = 0;
-    } else {
-      continue;     /* ignore unknown */
-    }
-    sprintf( fmt, "%%%ds", data_fields[fld_index[j]].length );
-    sprintf( buf +33+ len, fmt, data_fields[fld_index[j]].title );
-    len +=  data_fields[fld_index[j]].length;
+    j = fld_index[fld_active[i]];
+    if (j < 0) continue;
+
+    sprintf( fmt, "%%%ds", data_fields[j].length );
+    sprintf( buf +33+ len, fmt, data_fields[j].title );
+    len +=  data_fields[j].length;
   }
   printf("%s\n",buf);
 
@@ -92,27 +83,18 @@ void report_close() {
     len=0;
     sprintf( buf, " %2d. %-33s", at+1, name);
     for( i=0; i<MAXFLD; i++ ) {
-      if( fld_active[i]>= 'a' && fld_active[i]<= 'z') {
-        j = fld_active[i] - 'a' + 11 + 26;
-      } else if( fld_active[i]>= 'A' && fld_active[i]<= 'Z') {
-        j = fld_active[i] - 'A' + 11;
-      } else if( fld_active[i]>= '0' && fld_active[i]<= '9') {
-        j = fld_active[i] - '0' +1;
-      } else if( fld_active[i] == ' ' ) {
-        j = 0;
-      } else {
-        continue;     /* ignore stuff don't understand */
-      }
+      j = fld_index[fld_active [i]];
+      if (j < 0) continue;
 
       /* 1000.0 is a temporay hack for stats usec to ms, impacted net_loss. */
-      if( index( data_fields[ fld_index[j] ].format, 'f' ) ) {
-       sprintf( buf +33+ len, data_fields[ fld_index[j] ].format,
-               data_fields[ fld_index[j] ].net_xxx(at) /1000.0 );
+      if( index( data_fields[j].format, 'f' ) ) {
+       sprintf( buf +33+ len, data_fields[j].format,
+               data_fields[j].net_xxx(at) /1000.0 );
       } else {
-       sprintf( buf +33+ len, data_fields[ fld_index[j] ].format,
-               data_fields[ fld_index[j] ].net_xxx(at) );
+       sprintf( buf +33+ len, data_fields[j].format,
+               data_fields[j].net_xxx(at) );
       }
-      len +=  data_fields[fld_index[j]].length;
+      len +=  data_fields[j].length;
     }
     printf("%s\n",buf);
   }
@@ -165,33 +147,23 @@ void xml_close() {
 
     printf("    <HUB COUNT=%d HOST=%s>\n", at+1, name);
     for( i=0; i<MAXFLD; i++ ) {
-      if( fld_active[i]>= 'a' && fld_active[i]<= 'z') {
-        j = fld_active[i] - 'a' + 11 + 26;
-      } else if( fld_active[i]>= 'A' && fld_active[i]<= 'Z') {
-        j = fld_active[i] - 'A' + 11;
-      } else if( fld_active[i]>= '0' && fld_active[i]<= '9') {
-        j = fld_active[i] - '0' +1;
-      } else if( fld_active[i] == ' ' ) {
-        continue;     /* ignore space */
-        j = 0;
-      } else {
-        continue;     /* ignore stuff don't understand */
-      }
+      j = fld_index[fld_active[i]];
+      if (j < 0) continue;
 
       strcpy(name, "        <%s>");
-      strcat(name, data_fields[ fld_index[j] ].format);
+      strcat(name, data_fields[j].format);
       strcat(name, "</%s>\n");
       /* 1000.0 is a temporay hack for stats usec to ms, impacted net_loss. */
-      if( index( data_fields[ fld_index[j] ].format, 'f' ) ) {
+      if( index( data_fields[j].format, 'f' ) ) {
        printf( name,
-               data_fields[fld_index[j]].title,
-               data_fields[ fld_index[j] ].net_xxx(at) /1000.0,
-               data_fields[fld_index[j]].title );
+               data_fields[j].title,
+               data_fields[j].net_xxx(at) /1000.0,
+               data_fields[j].title );
       } else {
        printf( name,
-               data_fields[fld_index[j]].title,
-               data_fields[ fld_index[j] ].net_xxx(at),
-               data_fields[fld_index[j]].title );
+               data_fields[j].title,
+               data_fields[j].net_xxx(at),
+               data_fields[j].title );
       }
     }
     printf("    </HUB>\n");
@@ -224,19 +196,10 @@ void csv_close() {
   /* Header */
   printf("HUPCOUNT, HOST");
   for( i=0; i<MAXFLD; i++ ) {
-      if( fld_active[i]>= 'a' && fld_active[i]<= 'z') {
-        j = fld_active[i] - 'a' + 11 + 26;
-      } else if( fld_active[i]>= 'A' && fld_active[i]<= 'Z') {
-        j = fld_active[i] - 'A' + 11;
-      } else if( fld_active[i]>= '0' && fld_active[i]<= '9') {
-        j = fld_active[i] - '0' +1;
-      } else if( fld_active[i] == ' ' ) {
-        continue;     /* ignore space */
-        j = 0;
-      } else {
-        continue;     /* ignore stuff don't understand */
-      }
-      printf( ", %s", data_fields[fld_index[j]].title );
+      j = fld_index[fld_active[i]];
+      if (j < 0) continue; 
+
+      printf( ", %s", data_fields[j].title );
   }
   printf("\n");
 
@@ -262,24 +225,14 @@ void csv_close() {
 
     printf("%d, %s", at+1, name);
     for( i=0; i<MAXFLD; i++ ) {
-      if( fld_active[i]>= 'a' && fld_active[i]<= 'z') {
-        j = fld_active[i] - 'a' + 11 + 26;
-      } else if( fld_active[i]>= 'A' && fld_active[i]<= 'Z') {
-        j = fld_active[i] - 'A' + 11;
-      } else if( fld_active[i]>= '0' && fld_active[i]<= '9') {
-        j = fld_active[i] - '0' +1;
-      } else if( fld_active[i] == ' ' ) {
-        continue;     /* ignore space */
-        j = 0;
-      } else {
-        continue;     /* ignore stuff don't understand */
-      }
+      j = fld_index[fld_active[j]];
+      if (j < 0) continue; 
 
       /* 1000.0 is a temporay hack for stats usec to ms, impacted net_loss. */
-      if( index( data_fields[ fld_index[j] ].format, 'f' ) ) {
-       printf( ", %.2f", data_fields[ fld_index[j] ].net_xxx(at) /1000.0);
+      if( index( data_fields[j].format, 'f' ) ) {
+       printf( ", %.2f", data_fields[j].net_xxx(at) / 1000.0);
       } else {
-       printf( ", %d", data_fields[ fld_index[j] ].net_xxx(at) );
+       printf( ", %d",   data_fields[j].net_xxx(at) );
       }
     }
     printf("\n");