]> git.ipfire.org Git - thirdparty/bash.git/commitdiff
fix for nofork comsub command printing; fix for crash caused by tempvar assignment...
authorChet Ramey <chet.ramey@case.edu>
Tue, 20 Jun 2023 15:10:39 +0000 (11:10 -0400)
committerChet Ramey <chet.ramey@case.edu>
Tue, 20 Jun 2023 15:10:39 +0000 (11:10 -0400)
CWRU/CWRU.chlog
arrayfunc.c
arrayfunc.h
builtins/declare.def
print_cmd.c
shell.h
subst.h
variables.c

index 6cdfb14b0f2133c604e485bf6199d8b48d9c8b13..7ca101a7b971d0817a5aef4db60bc5547bed1cdf 100644 (file)
@@ -6711,3 +6711,43 @@ jobs.c
        - wait_for: make checkwinsize work in subshell commands started from
          interactive shells
          New feature request by Kerin Millar <kfm@plushkava.net>
+
+print_cmd.c
+       - semicolon: don't return immediately unless a preceding `&' is
+         itself preceded by a space, as the rest of the printing code prints
+         asynchronous commands.
+         Report and patch by Grisha Levit <grishalevit@gmail.com>
+
+                                  6/17
+                                  ----
+arrayfunc.c
+       - arrayvar_copyval: copy an array or hash table from shell variable
+         V1 to V2
+
+arrayfunc.h
+       - arrayvar_copyval: extern declaration
+
+subst.h
+       - ASS_NOTEMPENV: new flag for bind_variable and assignments
+
+variables.c
+       - bind_variable: if ASS_NOTEMPENV is in FLAGS, don't bother calling
+         bind_tempenv_variable
+
+builtins/declare.def
+       - declare_internal: if we find the variable in the temporary
+         environment, call bind_variable with the ASS_NOTEMPENV flag since
+         we are already dealing with a variable from the temporary environment
+       - declare_internal: if we bind a new global variable because we're
+         modifying a variable from the temporary environment, make sure to
+         call arrayvar_copyval to copy array variables from the temporary
+         environment to the global scope
+
+variables.c
+       - push_temp_var: if we are pushing an array variable, make sure to
+         call arrayvar_copyval on the new variable
+         Final fix for bug reported by Wiley Young <wyeth2485@gmail.com>
+       - push_posix_tempvar_internal: call arrayar_copyval instead of using
+         inline code
+
+
index 28aee54b804479e5e4a1f14315d24a6e4b6b34a3..74ff63166db428f1d96006d0f10fca28e2eda143 100644 (file)
@@ -142,6 +142,19 @@ convert_var_to_assoc (SHELL_VAR *var)
   return var;
 }
 
+/* Copy the array (ARRAY *) or assoc (HASH_TABLE *) from variable V1 to V2,
+   and return V2. */
+SHELL_VAR *
+arrayvar_copyval (SHELL_VAR *v1, SHELL_VAR *v2)
+{
+  FREE (value_cell (v2));
+  if (array_p (v1))
+    var_setarray (v2, array_copy (array_cell (v1)));
+  else if (assoc_p (v1))
+    var_setassoc (v2, assoc_copy (assoc_cell (v1)));
+  return v2;
+}
+
 char *
 make_array_variable_value (SHELL_VAR *entry, arrayind_t ind, const char *key, const char *value, int flags)
 {
index aa11101366044405c4409c80bb7a95cd245f77fc..08458c54218abc69f8c5fee03732ebed33f1c808 100644 (file)
@@ -73,6 +73,8 @@ extern int array_expand_once;
 extern SHELL_VAR *convert_var_to_array (SHELL_VAR *);
 extern SHELL_VAR *convert_var_to_assoc (SHELL_VAR *);
 
+extern SHELL_VAR *arrayvar_copyval (SHELL_VAR *, SHELL_VAR *);
+
 extern char *make_array_variable_value (SHELL_VAR *, arrayind_t, const char *, const char *, int);
 
 extern SHELL_VAR *bind_array_variable (const char *, arrayind_t, const char *, int);
index fd0ac44a211ca4fdcd574999187699a6e3775732..337e6755ec7acd102fa140f44be6f5c97a88b9d8 100644 (file)
@@ -1007,21 +1007,27 @@ restart_new_var_name:
       if ((flags_on & (att_exported|att_readonly)) && tempvar_p (var))
        {
          SHELL_VAR *tv;
-         char *tvalue;
 
          /* Temporary environment? Or in the local variable context? */
          tv = find_tempenv_variable (name_cell (var));
          if (tv)
            {
-             tvalue = var_isset (var) ? savestring (value_cell (var)) : savestring ("");
-             tv = bind_variable (name_cell (var), tvalue, 0);
+             /* We don't bother with modifying the temporary env because
+                we're already using it. */
+             tv = bind_variable (name_cell (var), value_cell (var), ASS_NOTEMPENV);
+
              if (tv)
                {
+#if defined (ARRAY_VARS)
+                 /* copy array value if array variable */
+                 if ((array_p (var) || assoc_p (var)))
+                   arrayvar_copyval (var, tv);
+#endif
+                 /* then copy attributes */
                  tv->attributes |= var->attributes & ~att_tempvar;
                  if (tv->context > 0 && local_p (var) == 0)    /* just paranoia here */
                    VSETATTR (tv, att_propagate);
                }
-             free (tvalue);
            }
          /* XXX - don't propagate local variables back to a previous scope,
             even in posix mode. */
index 298708378edd449c09f82f66482164f6e8093924..f8524e04ac094fe996aa5886a0c7326505b37197 100644 (file)
@@ -671,7 +671,7 @@ print_group_command (GROUP_COM *group_command)
   group_command_nesting++;
   cprintf ("{ ");
 
-  if (inside_function_def == 0)
+  if (inside_function_def == 0 /* && pretty_print_mode == 0 */)
     skip_this_indent++;
   else
     {
@@ -685,7 +685,7 @@ print_group_command (GROUP_COM *group_command)
   make_command_string_internal (group_command->command);
   PRINT_DEFERRED_HEREDOCS ("");
 
-  if (inside_function_def)
+  if (inside_function_def /* || pretty_print_mode */)
     {
       cprintf ("\n");
       indentation -= indentation_amount;
@@ -1446,9 +1446,11 @@ indent (int amount)
 static void
 semicolon (void)
 {
-  if (command_string_index > 0 &&
-       (the_printed_command[command_string_index - 1] == '&' ||
-       the_printed_command[command_string_index - 1] == '\n'))
+  if ((command_string_index > 0 &&
+       the_printed_command[command_string_index - 1] == '\n') ||
+      (command_string_index > 1 && 
+       the_printed_command[command_string_index - 1] == '&' &&
+       the_printed_command[command_string_index - 2] == ' '))
     return;
   cprintf (";");
 }
diff --git a/shell.h b/shell.h
index 62c09028b07f13e24a58cc18dbb8bd7025ecd713..055ba9f50dd534b41f9d1cfc80ac9fb5e9ed3936 100644 (file)
--- a/shell.h
+++ b/shell.h
@@ -109,6 +109,8 @@ extern int indirection_level;
 extern int shell_compatibility_level;
 extern int running_under_emacs;
 
+extern int pretty_print_mode;
+
 extern int posixly_correct;
 extern int no_line_editing;
 
diff --git a/subst.h b/subst.h
index c7e948f1880a578f7ee81458bf654e42a2d56547..58ab077b57fe1ba334383e7ec7db2824fd1feb5b 100644 (file)
--- a/subst.h
+++ b/subst.h
@@ -57,6 +57,7 @@
 #define ASS_NOINVIS    0x0400  /* don't resolve local invisible variables */
 #define ASS_ALLOWALLSUB        0x0800  /* allow * and @ as associative array keys */
 #define ASS_ONEWORD    0x1000  /* don't check array subscripts, assume higher level has done that */
+#define ASS_NOTEMPENV  0x2000  /* don't assign into temporary environment */
 
 /* Flags for the string extraction functions. */
 #define SX_NOALLOC     0x0001  /* just skip; don't return substring */
index 17a535b2dbfb954f594d6d45cdaf37b3a89c0437..2db4a457c7cc63137f094b00f747eb146451137f 100644 (file)
@@ -283,6 +283,8 @@ static SHELL_VAR *new_shell_variable (const char *);
 static SHELL_VAR *make_new_variable (const char *, HASH_TABLE *);
 static SHELL_VAR *bind_variable_internal (const char *, const char *, HASH_TABLE *, int, int);
 
+static void init_shell_variable (SHELL_VAR *);
+
 static void dispose_variable_value (SHELL_VAR *);
 static void free_variable_hash_data (PTR_T);
 
@@ -2657,6 +2659,21 @@ make_local_variable (const char *name, int flags)
     new_var = make_new_variable (name, vc->table);
   else
     {
+#if 0
+      /* This handles the case where a variable is found in both the temporary
+        environment *and* declared as a local variable. If we want to avoid
+        multiple entries with the same name in VC->table (that might mess up
+        unset), we need to use the existing variable entry and destroy the
+        current value. Currently disabled because it doesn't matter -- the
+        right things happen. */
+      new_var = 0;
+      if (was_tmpvar && (new_var = hash_lookup (name, vc->table)))
+       {
+         dispose_variable_value (new_var);
+         init_variable (new_var);
+       }
+      if (new_var == 0)
+#endif
       new_var = make_new_variable (name, vc->table);
 
       /* If we found this variable in one of the temporary environments,
@@ -2716,15 +2733,9 @@ set_local_var_flags:
   return (new_var);
 }
 
-/* Create a new shell variable with name NAME. */
-static SHELL_VAR *
-new_shell_variable (const char *name)
+static void
+init_variable (SHELL_VAR *entry)
 {
-  SHELL_VAR *entry;
-
-  entry = (SHELL_VAR *)xmalloc (sizeof (SHELL_VAR));
-
-  entry->name = savestring (name);
   var_setvalue (entry, (char *)NULL);
   CLEAR_EXPORTSTR (entry);
 
@@ -2737,7 +2748,18 @@ new_shell_variable (const char *name)
      make_local_variable has the responsibility of changing the
      variable context. */
   entry->context = 0;
+}
 
+/* Create a new shell variable with name NAME. */
+static SHELL_VAR *
+new_shell_variable (const char *name)
+{
+  SHELL_VAR *entry;
+
+  entry = (SHELL_VAR *)xmalloc (sizeof (SHELL_VAR));
+
+  entry->name = savestring (name);
+  init_variable (entry);
   return (entry);
 }
 
@@ -3202,8 +3224,9 @@ bind_variable (const char *name, const char *value, int flags)
      and, if found, modify the value there before modifying it in the
      shell_variables table.  This allows sourced scripts to modify values
      given to them in a temporary environment while modifying the variable
-     value that the caller sees. */
-  if (temporary_env && value)          /* XXX - can value be null here? */
+     value that the caller sees. The caller can inhibit this by setting
+     ASS_NOTEMPENV in FLAGS. */
+  if (temporary_env && value && (flags & ASS_NOTEMPENV) == 0)  /* XXX - can value be null here? */
     bind_tempenv_variable (name, value);
 
   /* XXX -- handle local variables here. */
@@ -4493,6 +4516,11 @@ push_temp_var (PTR_T data)
 
   v = bind_variable_internal (var->name, value_cell (var), binding_table, 0, ASS_FORCE|ASS_NOLONGJMP);
 
+#if defined (ARRAY_VARS)
+  if (v && (array_p (var) || assoc_p (var)))
+    arrayvar_copyval (var, v);
+#endif
+
   /* XXX - should we set the context here?  It shouldn't matter because of how
      assign_in_env works, but we do it anyway. */
   if (v)
@@ -5174,6 +5202,7 @@ push_posix_tempvar_internal (SHELL_VAR *var, int isbltin)
       set_current_options (value_cell (var));
       set_shellopts ();
     }
+
   /* This takes variable assignments preceding special builtins that can execute
      multiple commands (source, eval, etc.) and performs the equivalent of
      an assignment statement to modify the closest enclosing variable (the
@@ -5215,14 +5244,8 @@ push_posix_tempvar_internal (SHELL_VAR *var, int isbltin)
 
 #if defined (ARRAY_VARS)
   if (v && (array_p (var) || assoc_p (var)))
-    {
-      FREE (value_cell (v));
-      if (array_p (var))
-       var_setarray (v, array_copy (array_cell (var)));
-      else
-       var_setassoc (v, assoc_copy (assoc_cell (var)));
-    }
-#endif   
+    arrayvar_copyval (var, v);
+#endif
 
   dispose_variable (var);
 }