[quagga-dev 4258] Re: vty patch to prevent infinitely recursing error messages

[Andrew J. Schorr]

On Mon, Jul 10, 2006 at 10:42:10PM +0100, Paul Jakma wrote:
> On Mon, 10 Jul 2006, Andrew J. Schorr wrote:
> 
> >I would argue that setting vty->monitor to 0 before calling 
> >zlog_warn ensures that the warning message will not be logged to 
> >this vty, so no worries there.  Do you see it differently?
> 
> Hmm, ok, I'm not familiar with the implications of that, where does 
> it happen? is that in your previously pending patch?

Yes, that was in the previous patch I committed earlier.  Each time
there is an I/O error, I set vty->monitor to 0 immediately before
proceeding.  That fixes the infinite recursion of error messages
problem by telling zlog not to send messages to this vty.

> I don't understand the vty->monitor/zlog_warn thing ;), but the rest 
> of it - yep, seems quite safe.

Perhaps this bears more explanation?  Let me know what further info
I can provide.  Does your version of lib/vty.c include the patch
I committed earlier?

> Maybe also document vty_close() that it should only be called as 
> close to the top of the "stack" for whichever vty thread is being 
> run, to avoid accidental re-use by intermediate functions.. ;)

OK, the attached patch adds a comment to vty_close.  Should that be
in the header file also?

Regards,
Andy
-------------- next part --------------
Index: lib/vty.c
===================================================================
RCS file: /var/cvsroot/quagga/lib/vty.c,v
retrieving revision 1.54
diff -b -u -p -r1.54 vty.c
--- lib/vty.c	10 Jul 2006 18:09:42 -0000	1.54
+++ lib/vty.c	10 Jul 2006 21:49:25 -0000
@@ -186,7 +186,10 @@ vty_log_out (struct vty *vty, const char
       zlog_warn("%s: write failed to vty client fd %d, closing: %s",
 		__func__, vty->fd, safe_strerror(errno));
       buffer_reset(vty->obuf);
-      vty_close(vty);
+      /* cannot call vty_close, because a parent routine may still try
+         to access the vty struct */
+      vty->status = VTY_CLOSE;
+      shutdown(vty->fd, SHUT_RDWR);
       return -1;
     }
   return 0;
@@ -2141,7 +2144,10 @@ vty_serv_sock (const char *addr, unsigne
 #endif /* VTYSH */
 }
 
-/* Close vty interface. */
+/* Close vty interface.  Warning: call this only from functions that
+   will be careful not to access the vty afterwards (since it has
+   now been freed).  This is safest from top-level functions (called
+   directly by the thread dispatcher). */
 void
 vty_close (struct vty *vty)
 {