local, rename: don't allow renaming a local proc with upcall

If an existing command was replaced when using local proc ...,
then renaming the command will break upcall from within the proc
because the command no longer exists in the namespace. To prevent
this case, disallow rename of such a command.

It is still OK to delete with rename <cmd> ""

Signed-off-by: Steve Bennett <steveb@workware.net.au>

2 files changed, 37 insertions, 10 deletions

diff --git a/jim.c b/jim.c
index 6546903..e754711 100644
--- a/jim.c
+++ b/jim.c
@@ -4120,19 +4120,27 @@ int Jim_RenameCommand(Jim_Interp *interp, const char *oldName, const char *newNa
         Jim_SetResultFormatted(interp, "can't rename to \"%s\": command already exists", newName);
     }
     else {
-        /* Add the new name first */
         cmdPtr = Jim_GetHashEntryVal(he);
-        JimIncrCmdRefCount(cmdPtr);
-        JimUpdateProcNamespace(interp, cmdPtr, fqnew);
-        Jim_AddHashEntry(&interp->commands, fqnew, cmdPtr);
+        if (cmdPtr->prevCmd) {
+            /* If the command replaced another command with 'local', renaming it
+             * would break the usage of upcall, so don't allow it.
+             */
+            Jim_SetResultFormatted(interp, "can't rename local command \"%s\"", oldName);
+        }
+        else {
+            /* Add the new name first */
+            JimIncrCmdRefCount(cmdPtr);
+            JimUpdateProcNamespace(interp, cmdPtr, fqnew);
+            Jim_AddHashEntry(&interp->commands, fqnew, cmdPtr);
 
-        /* Now remove the old name */
-        Jim_DeleteHashEntry(&interp->commands, fqold);
+            /* Now remove the old name */
+            Jim_DeleteHashEntry(&interp->commands, fqold);
 
-        /* Increment the epoch */
-        Jim_InterpIncrProcEpoch(interp);
+            /* Increment the epoch */
+            Jim_InterpIncrProcEpoch(interp);
 
-        ret = JIM_OK;
+            ret = JIM_OK;
+        }
     }
 
     JimFreeQualifiedName(interp, qualifiedOldNameObj);
diff --git a/tests/alias.test b/tests/alias.test
index 4f6e553..866aa1b 100644
--- a/tests/alias.test
+++ b/tests/alias.test
@@ -240,9 +240,28 @@ test upcall-1.4 "upcall errors" {
 	list [catch {upcall a} msg] $msg
 } {1 {no previous command: "a"}}
 
-test upcall-1.4 "upcall errors" {
+test upcall-1.5 "upcall errors" {
 	proc a {} {upcall a}
 	list [catch a msg] $msg
 } {1 {no previous command: "a"}}
 
+test upcall-1.6 "delete local command" -setup {
+	# First make sure a is gone
+	rename a ""
+} -body {
+	local proc a {x} {list 2 $x}
+	# It is OK to rename this local proc
+	rename a b
+	b 5
+} -result {2 5} -cleanup {
+	rename b ""
+}
+
+test upcall-1.6 {delete local command with upcall} -body {
+	local proc a {x} {list 2 $x}
+	local proc a {x} {list 3 $x}
+	# Can't rename a because it would invalide upcalls from a
+	rename a c
+} -returnCodes error -result {can't rename local command "a"}
+
 testreport