[Qemacs-commit] qemacs shell.c qe.h qe.c

[Charlie Gordon]

CVSROOT:        /sources/qemacs
Module name:    qemacs
Changes by:     Charlie Gordon <chqrlie>        17/05/07 16:32:31

Modified files:
        .              : shell.c qe.h qe.c 

Log message:
        shell: fix spurious crash bug
        - add check_mode_data() to verify mode data pointer validity
        - avoid double free in qe_free_mode_data() and shell_pid_cb()

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/qemacs/shell.c?cvsroot=qemacs&r1=1.131&r2=1.132
http://cvs.savannah.gnu.org/viewcvs/qemacs/qe.h?cvsroot=qemacs&r1=1.261&r2=1.262
http://cvs.savannah.gnu.org/viewcvs/qemacs/qe.c?cvsroot=qemacs&r1=1.281&r2=1.282

Patches:
Index: shell.c
===================================================================
RCS file: /sources/qemacs/qemacs/shell.c,v
retrieving revision 1.131
retrieving revision 1.132
diff -u -b -r1.131 -r1.132
--- shell.c     3 May 2017 07:18:05 -0000       1.131
+++ shell.c     7 May 2017 20:32:30 -0000       1.132
@@ -1627,6 +1627,7 @@
             for (i = 0; i < s->nb_params; i++) {
                 switch (s->params[i]) {
                 case 1:     /* Application Cursor Keys (DECCKM). */
+                case 3:     /* Set Number of Columns to 132 (DECCOLM) */
                 case 4:     /* Smooth (Slow) Scroll (DECSCLM). */
                     break;
                 case 5:     /* Reverse Video (DECSCNM) */
@@ -1712,6 +1713,7 @@
             for (i = 0; i < s->nb_params; i++) {
                 switch (s->params[i]) {
                 case 1:     /* Normal Cursor Keys (DECCKM). */
+                case 3:     /* Set Number of Columns to 80 (DECCOLM) */
                 case 4:     /* Jump (Fast) Scroll (DECSCLM). */
                     break;
                 case 5:     /* Normal Video (DECSCNM). */
@@ -1942,12 +1944,17 @@
 
 static void shell_pid_cb(void *opaque, int status)
 {
-    ShellState *s = opaque;
+    ShellState *s;
     EditBuffer *b;
     QEmacsState *qs;
     EditState *e;
     char buf[1024];
 
+    /* Extra check in case mode data was freed already.
+     * It is not completely fool proof as the same address might have
+     * been reallocated for the same purpose, quite unlikely.
+     */
+    s = check_mode_data(&opaque);
     if (!s || s->base.mode != &shell_mode)
         return;
 
@@ -2007,6 +2014,8 @@
             qe_set_next_mode(e, 0, 0);
     }
     if (!(s->shell_flags & SF_INTERACTIVE)) {
+        /* Must Unlink the shell data to avoid potential crash */
+        //shell_mode_free(b, s);  // called by qe_free_mode_data
         qe_free_mode_data(&s->base);
     }
     edit_display(qs);
@@ -2082,6 +2091,7 @@
         return NULL;
     }
 
+    /* XXX: ShellState life cycle is bogus */
     set_read_handler(s->pty_fd, shell_read_cb, s);
     set_pid_handler(s->pid, shell_pid_cb, s);
     return b;

Index: qe.h
===================================================================
RCS file: /sources/qemacs/qemacs/qe.h,v
retrieving revision 1.261
retrieving revision 1.262
diff -u -b -r1.261 -r1.262
--- qe.h        5 May 2017 20:08:52 -0000       1.261
+++ qe.h        7 May 2017 20:32:30 -0000       1.262
@@ -1572,6 +1572,7 @@
 void *qe_get_buffer_mode_data(EditBuffer *b, ModeDef *m, EditState *e);
 QEModeData *qe_create_window_mode_data(EditState *s, ModeDef *m);
 void *qe_get_window_mode_data(EditState *e, ModeDef *m, int status);
+void *check_mode_data(void **pp);
 int qe_free_mode_data(QEModeData *md);
 
 /* from tty.c */

Index: qe.c
===================================================================
RCS file: /sources/qemacs/qemacs/qe.c,v
retrieving revision 1.281
retrieving revision 1.282
diff -u -b -r1.281 -r1.282
--- qe.c        7 May 2017 06:09:19 -0000       1.281
+++ qe.c        7 May 2017 20:32:30 -0000       1.282
@@ -2146,6 +2146,26 @@
     return NULL;
 }
 
+void *check_mode_data(void **pp) {
+    QEmacsState *qs = &qe_state;
+    QEModeData *md = *pp;
+    EditBuffer *b;
+    EditState *e;
+
+    for (b = qs->first_buffer; b != NULL; b = b->next) {
+        QEModeData **mdp;
+        for (mdp = &b->mode_data_list; *mdp; mdp = &(*mdp)->next) {
+            if (*mdp == md)
+                return md;
+        }
+    }
+    for (e = qs->first_window; e != NULL; e = e->next_window) {
+        if (e->mode_data == md)
+            return md;
+    }
+    return NULL;
+}
+
 int qe_free_mode_data(QEModeData *md)
 {
     int rc = -1;
@@ -2173,7 +2193,10 @@
             rc = 0;
         }
     }
+    if (rc == 0) {
+        /* mode data was found, OK to free */
     qe_free(&md);
+    }
     return rc;
 }
 
@@ -5773,6 +5796,7 @@
         /* save current state for later window reattachment */
         switch_to_buffer(s, NULL);
         edit_detach(s);
+        /* closing the window mode should have freed it already */
         qe_free_mode_data(s->mode_data);
         qe_free(&s->prompt);
         qe_free(&s->line_shadow);