[Plplot-cvs] SF.net SVN: plplot:[13043] trunk/bindings/tk/plserver.c

[<ai...@us...>]

Revision: 13043
          http://sourceforge.net/p/plplot/code/13043
Author:   airwin
Date:     2014-03-05 20:19:04 +0000 (Wed, 05 Mar 2014)
Log Message:
-----------
Fix long-standing memory management issue discovered accidentally by a
valgrind run on plserver.

The argv argument to main is an array of argc + 1 pointers.  The first
argc of these point to NULL-terminated strings while the last is a
NULL pointer (according to both Linux and Windows documentation of
main that I have read).

Tk_ParseArgv follows this same standard (i.e., writes a NULL
pointer as the argc element of the argv array) when it modifies argc and argv.

Previously there was only room in myargv for myargc = original argc elements, but
that has now been changed to argc+1, and that solves the memory
management issue as expected.

Writing a NULL just off the end of an allocated block of memory in the
heap is generally not a good thing to do and this fix addresses that
issue.  However, plserver has been reliable before this fix so it
appears we were by accident escaping unscathed from this memory
management issue.  So the practical effect of this fix will not be
apparent at the moment, but plserver should be more robust against
further changes which might have generated a segfault without the
present memory management fix.

Modified Paths:
--------------
    trunk/bindings/tk/plserver.c

Modified: trunk/bindings/tk/plserver.c
===================================================================
--- trunk/bindings/tk/plserver.c	2014-03-05 01:54:00 UTC (rev 13042)
+++ trunk/bindings/tk/plserver.c	2014-03-05 20:19:04 UTC (rev 13043)
@@ -117,8 +117,11 @@
     fprintf( stderr, "Before myargv\n" );
 #endif
 
-    myargv = (const char **) malloc( argc * sizeof ( char * ) );
-    for ( i = 0; i < argc; i++ )
+    // According to both Linux and Windows documentation,
+    // argv is actually argc+1 in length with the last element pointing
+    // to NULL.  So leave room for that.
+    myargv = (const char **) malloc( ( argc + 1 ) * sizeof ( char * ) );
+    for ( i = 0; i < argc + 1; i++ )
     {
         myargv[i] = argv[i];
     }
@@ -133,12 +136,16 @@
     if ( Tk_ParseArgv( interp, (Tk_Window) NULL, &argc, argv,
              argTable, TK_ARGV_NO_DEFAULTS ) != TCL_OK )
     {
+#ifdef DEBUG
         fprintf( stderr, "Error in Tk_ParseArgv\n" );
+#endif
         fprintf( stderr, "\n(plserver) %s\n\n", Tcl_GetStringResult( interp ) );
         fprintf( stderr, "\
 The client_<xxx> and -child options should not be used except via the\n\
 PLplot/Tk driver.\n\n(wish) " );
+#ifdef DEBUG
         fprintf( stderr, "Before Tcl_SetResult\n" );
+#endif
         Tcl_SetResult( interp, (char *) helpmsg, TCL_VOLATILE );
     }
 

This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.