This is the mail archive of the insight@sourceware.org mailing list for the Insight project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Remove deprecated access to tcl internal variables


I noticed this a while back, too.. Some small nits.

In the future, could you please
add "-p" option to diff? I have "diff -upN" set in my .cvsrc. It gives me a little more context when reading the patch.


On 03/19/2012 04:45 AM, Roland Schwingel wrote:
--- gdbtk_orig/generic/gdbtk.c	2012-03-19 11:21:15.542232400 +0100
+++ gdbtk/generic/gdbtk.c	2012-03-19 11:23:12.099170600 +0100
@@ -494,17 +494,17 @@
    make_final_cleanup (gdbtk_cleanup, NULL);

    if (Tcl_Init (gdbtk_interp) != TCL_OK)
-    error ("Tcl_Init failed: %s", gdbtk_interp->result);
+    error ("Tcl_Init failed: %s", Tcl_GetStringResult(gdbtk_interp));

Watch the spaces in between function names and open parenthesis. This is a GNU convention (which we follow). That should be "Tcl_GetStringResult (gdbtk_interp)".


There are several places where this occurs throughout the patches (and subsequent ones).

tcl_compat_gdbtk-hooks.c.patch


--- gdbtk_orig/generic/gdbtk-hooks.c 2012-01-03 13:26:56.000000000 +0100 +++ gdbtk/generic/gdbtk-hooks.c 2012-03-05 11:47:03.340565000 +0100 @@ -254,17 +254,22 @@ { report_error (); actual_len = 0; + buf[0] = '\0'; + return 0;

This hunk is not mentioned in the ChangeLog.


I don't understand this. If we return 0 here, that means that no bytes in the buffer were filled-in, yet we are actually filling in one byte anyway. Is there a problem this was designed to solve? Is the return value not being checked somewhere or buf assumed to have at least the string terminal in it?

  	}
        else
-        actual_len = strlen (gdbtk_interp->result);
+      {
+      	const char *tclResult = Tcl_GetStringResult(gdbtk_interp);
+        actual_len = strlen (tclResult);

        /* Truncate the string if it is too big for the caller's buffer.  */
        if (actual_len>= sizeof_buf)
  	actual_len = sizeof_buf - 1;

-      memcpy (buf, gdbtk_interp->result, actual_len);
+      memcpy (buf,tclResult, actual_len);
        buf[actual_len] = '\0';
        return actual_len;
+     }
      }
    else
      {

Keith



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]