[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

fix for Watches window



Just out of curiosity, am I the only one who wants to use watch
expressions, or why has no one ever bothered to fix XWPE's Watches window?
It's been broken (in the sense that XWPE would crash rather sooner than
later once you started using Watches) as long as I can remember (I think I
tried XWPE for the first time 6 years ago). 
Anyway, I fixed it. Please apply the attached patch so that other users
can finally start watching. And please do not strip out the comments. If
there's one thing the xwpe code desperately needs, it's comments. I find
it hard enough to debug code I've never seen before. Uncommented global
variables with utterly misleading names (e_d_nrwtchs) do NOT make things
easier :-)

MSB

-- 
Indecision is the key to flexibility.

diff -Naur xwpe-1.5.29a/edit.h xwpe-1.5.29a-patched/edit.h
--- xwpe-1.5.29a/edit.h	Wed Jun  5 04:53:50 2002
+++ xwpe-1.5.29a-patched/edit.h	Sat Jan 11 17:50:52 2003
@@ -156,8 +156,8 @@
 } STRING;
 
 typedef struct BFF {
- STRING *bf;
- POINT b;
+ STRING *bf; /* bf[i] is the i-th line of the buffer */
+ POINT b;    /* cursor coordinates in window (at least in some contexts) */
  POINT mx; /* maximum column and line */
  int mxlines; /* number of lines */
  int cl, clsv;
diff -Naur xwpe-1.5.29a/unixmakr.h xwpe-1.5.29a-patched/unixmakr.h
--- xwpe-1.5.29a/unixmakr.h	Wed Jun  5 04:53:50 2002
+++ xwpe-1.5.29a-patched/unixmakr.h	Sat Jan 11 17:03:00 2003
@@ -93,7 +93,7 @@
 
 #define REALLOC(p, n) realloc((p), (n))
 #define MALLOC(n) malloc(n)
-#define FREE(n) free(n)
+#define FREE(n) do{free(n);n=NULL;}while(0)
 
 #ifdef NEWSTYLE
 extern char *extbyte, *altextbyte;
diff -Naur xwpe-1.5.29a/we_debug.c xwpe-1.5.29a-patched/we_debug.c
--- xwpe-1.5.29a/we_debug.c	Wed Jun  5 04:53:50 2002
+++ xwpe-1.5.29a-patched/we_debug.c	Sat Jan 11 18:21:37 2003
@@ -36,9 +36,19 @@
 
 char *e_debugger, *e_deb_swtch = NULL, *e_d_save_schirm;
 int e_d_swtch = 0, rfildes[2], ofildes, e_d_pid = 0;
-int e_d_nbrpts = 0, e_d_nwtchs = 0, e_d_zwtchs = 0, *e_d_ybrpts, *e_d_nrbrpts, *e_d_nrwtchs;
+int e_d_nbrpts = 0, e_d_zwtchs = 0, *e_d_ybrpts, *e_d_nrbrpts;
+
+/* number of watch expressions in Watches window */
+int e_d_nwtchs = 0; 
+
+/* e_d_nrwtchs[i] is the y coordinate (count starts at 0) of the first line of
+   the i-th watch in the Watches window */
+int *e_d_nrwtchs; 
+
+char **e_d_swtchs; /* e_d_swtchs[i] is the i-th watch expression (a char*) */
+
 int e_d_nstack;
-char e_d_file[128], **e_d_sbrpts, **e_d_swtchs;
+char e_d_file[128], **e_d_sbrpts;
 char *e_d_out = NULL;
 int e_deb_type = 0, e_deb_mode = 0;
 char e_d_out_str[SVLINES][256];
@@ -575,12 +585,17 @@
 {
  char str[128];
  int i, y;
-
+/* the following loop looks like it's equivalent to
+   i= ((f->ed->mxedt <= 0)||(strcmp(f->datnam,"Watches")==0)) ? f->ed->mxedt:0
+   Is e_make_watches ever called for a window other than the Watches window?
+   If not, this is the same as i=f->ed->mxedt
+   The same thing occurs elsewhere. 
+ */  
  for (i = f->ed->mxedt; i > 0 && strcmp(f->datnam, "Watches"); i--)
   ;
- if (i > 0)
- {
-  for(y = 0; y < e_d_nwtchs && e_d_nrwtchs[y] < f->ed->f[i]->b->b.y; y++)
+ if (i > 0) /* probably the same as if (f->ed->mxedt>0) because of the above */
+ { /* sets y=number of watch we're inserting */
+  for(y = 0; y < e_d_nwtchs && e_d_nrwtchs[y]   <=   f->ed->f[i]->b->b.y; y++)
    ;
  }
  else
@@ -602,13 +617,22 @@
    e_d_swtchs = REALLOC(e_d_swtchs, e_d_nwtchs * sizeof(char *));
    e_d_nrwtchs = REALLOC(e_d_nrwtchs, e_d_nwtchs * sizeof(int));
   }
+  
+  /*
+    move watch number y and following up one position so that we can insert 
+    at position y 
+  */
   for (i = e_d_nwtchs - 1; i > y; i--)
   {
    e_d_swtchs[i] = e_d_swtchs[i-1];
+   
+   /* The following instruction is pointless as e_d_nrwtchs[i] is invalidated
+      by inserting the new watch and has to be recomputed by e_d_p_watches()
+   */
    e_d_nrwtchs[i] = e_d_nrwtchs[i-1];
   }
-  e_d_swtchs[y] = MALLOC(strlen(str) + 1);
-  strcpy(e_d_swtchs[y], str);
+  e_d_swtchs[y] = MALLOC(strlen(str) + 1); /* insert...                    */
+  strcpy(e_d_swtchs[y], str);              /*       ... new watch at pos y */
   e_d_p_watches(f, 1);
   return(0);
  }
@@ -638,21 +662,24 @@
  return(-1);
 }
 
+/* Among other things, e_d_p_watches() must recompute e_d_nrwtchs when
+   called from e_edit_watches(), 
+   but has code paths that don't do this ==> possible BUG
+*/
 int e_d_p_watches(FENSTER *f, int sw)
 {
  ECNT *cn = f->ed;
  BUFFER *b;
  SCHIRM *s;
  int iw, i, k = 0, l, ret;
- char str1[256], *str = str1;
- char *str2;
 
  e_d_switch_out(0);
  if ((e_d_swtch > 2) && (e_d_p_stack(f, 0) == -1))
   return(-1);
+ /* Same weird loop as the one to determine i in e_make_watches() */
  for (iw = cn->mxedt; iw > 0 && strcmp(cn->f[iw]->datnam, "Watches"); iw--);
  if (iw == 0 && !e_d_nwtchs)
- {
+ { /* if no watches and the mysterious iw!=0 then just repaint window tree */
   e_rep_win_tree(cn);
   return(0);
  }
@@ -670,12 +697,17 @@
  f = cn->f[iw];
  b = cn->f[iw]->b;
  s = cn->f[iw]->s;
+ 
+ /* free all lines of BUFFER b */
  e_p_red_buffer(b);
  FREE(b->bf[0].s);
  b->mxlines=0;
 
  for (i = 0, l = 0; l < e_d_nwtchs; l++)
  {
+  char str1[256], *str = str1; /* is 256 always large enough? */
+  char *str2;
+  
   /* Create appropriate command for the debugger */
   if (e_deb_type == 0 || e_deb_type == 3)
   {
@@ -706,13 +738,13 @@
   {
    if (ret == -1)
    {
-    return(ret);
+    return(ret); /* BUG? e_d_nrwtchs not initialized if this return is taken */
    }
    str = MALLOC(strlen(str1) + 1);
    strcpy(str, str1);
    while ((ret = e_d_line_read(wfildes[0], str1, 256, 0, 0)) == 0 || ret == 2)
    {
-    if (ret == -1) return(ret);
+    if (ret == -1) return(ret); /* BUG? e_d_nrwtchs not initialized if this return is taken */
     if (ret == 2) e_d_error(str1);
     str = REALLOC(str, (k = strlen(str)) + strlen(str1) + 1);
     if (str[k-1] == '\n') str[k-1] = ' ';
@@ -751,8 +783,8 @@
   str2 = WpeMalloc(strlen(e_d_swtchs[l]) + strlen(str + k) + 4);
   sprintf(str2, "%s: %s", e_d_swtchs[l], str + k);
 
+  e_d_nrwtchs[l] = b->mxlines;
   print_to_end_of_buffer(b, str2, b->mx.x);
-  e_d_nrwtchs[l] = b->mxlines-1;
 
   /* Free any allocated string */
   WpeFree(str2);
diff -Naur xwpe-1.5.29a/we_edit.c xwpe-1.5.29a-patched/we_edit.c
--- xwpe-1.5.29a/we_edit.c	Wed Jun  5 04:53:50 2002
+++ xwpe-1.5.29a-patched/we_edit.c	Sat Jan 11 18:08:23 2003
@@ -1503,10 +1503,11 @@
  int i, j;
 
  if (!DTMD_ISTEXT(f->dtmd)) return;
- if (b->b.y < 0) b->b.y = 0;
  if (b->b.y > b->mxlines-1) b->b.y = b->mxlines-1;
+ if (b->b.y < 0) b->b.y = 0;
  if (b->b.x < 0) b->b.x = 0;
- if (b->b.x > b->bf[b->b.y].len) b->b.x = b->bf[b->b.y].len;
+ if (b->mxlines==0) b->b.x=0; /* the else branch needs b->b.y < b->mxlines, which is not true for b->mxlines==0 */
+   else if (b->b.x > b->bf[b->b.y].len) b->b.x = b->bf[b->b.y].len;
  for (i = j = 0; i < b->b.x; i++)
  {
   if (*(b->bf[b->b.y].s + i) == WPE_TAB)
diff -Naur xwpe-1.5.29a/we_prog.c xwpe-1.5.29a-patched/we_prog.c
--- xwpe-1.5.29a/we_prog.c	Wed Jun  5 04:53:50 2002
+++ xwpe-1.5.29a-patched/we_prog.c	Sat Jan 11 18:23:14 2003
@@ -2761,13 +2761,14 @@
  return(ret);
 }
 
+/* After this function b has exactly 1 line allocated (b->mxlines==1).
+   This line is initialized to the string WPE_WR,0 */
 int e_p_red_buffer(BUFFER *b)
 {
  int i;
 
- for (i = 1; i < b->mxlines; i++)
-  if (b->bf[i].s != NULL)
-   FREE( b->bf[i].s );
+ for (i = 1; i < b->mxlines; i++) FREE( b->bf[i].s );
+ if (b->mxlines==0) e_new_line(0,b);
  b->bf[0].s[0] = WPE_WR;
  b->bf[0].s[1] = '\0';
  b->bf[0].len = 0;