fix issue with interupting timed functions; fix for parsing comsubs inside arith for commands; allow break and continue inside arith for expressions; fix for case-insensitive completion with multibyte chars; fix some minor memory leaks; reset read buffer if it returns partial results on EINTR; fix for undo in execute-named-command

CWRU/CWRU.chlog

@@ -9521,3 +9521,72 @@ execute_cmd.c
 	- shell_execve: fix typo in code that chops \r off the end of the #!
 	  interpreter
 	  Report and fix from Collin Funk <collin.funk1@gmail.com>
+
+				   5/28
+				   ----
+execute_cmd.c
+	- time_command: only restore command->flags if we haven't longjmped
+	  Report from Michael Maurer <michael.maurer@univie.ac.at> and
+	  Grisha Levit <grishalevit@gmail.com>
+
+subst.c
+	- skip_to_delim: if we're not skipping over command substitutions
+	  lexically, call extract_command_subst instead of using the old
+	  extract_delimited_string
+	  Fixes bug reported by Oguz <oguzismailuysal@gmail.com>
+
+execute_cmd.c
+	- execute_arith_for_command: handle the extremely unlikely case that
+	  the step or test expressions execute `break' or `continue' in a
+	  nofork command substitution
+	  Report from Oguz <oguzismailuysal@gmail.com>
+
+				   5/29
+				   ----
+lib/readline/complete.c
+	- compute_lcd_of_matches: if we have multiple matches that compare
+	  equally when using case-insensitive completion, but are different
+	  lengths due to the presence of multibyte characters, use the
+	  shorter match as the common prefix to avoid overflow
+	  Report from Grisha Levit <grishalevit@gmail.com>
+
+				   5/30
+				   ----
+lib/readline/text.c
+	- _rl_readstr_init: don't call rl_maybe_replace_line since we're not
+	  actually moving off this history line
+	  Report from Grisha Levit <grishalevit@gmail.com>
+
+builtins/read.def
+	- read_builtin: free ifs_chars in more return code paths
+	  Report and patch from Grisha Levit <grishalevit@gmail.com>
+
+				   5/31
+				   ----
+quit.h
+	- ZRESET: new macro, calls zreset() if interrupt_state is non-zero
+
+lib/sh/zread.c
+	- zread: if read returns -1/EINTR, and we're executing a builtin,
+	  call zreset just in case
+
+builtins/read.def
+	- read_builtin: if read returns > 0 (partial read) but interrupt_state
+	  is non-zero, we're going to call throw_to_top_level, so call
+	  ZRESET before that happens
+	  Report from Oguz <oguzismailuysal@gmail.com>
+	- read_builtin: if zread/zreadc/zreadintr/zreadcintr return -1/EINTR,
+	  make sure we call ZRESET in case zread did not
+
+variables.h
+	- ASSIGN_DISALLOWED: macro that encapsulates when an assignment to
+	  SHELL_VAR *v with flags f will be disallowed and fail
+
+arrayfunc.c,execute_cmd.c,expr.c,redir.c,subst.c,variables.c
+builtins/delare.def,builtins/getopts.def,builtins/printf.def,builtins/read.def
+	- use ASSIGN_DISALLOWED where appropriate
+
+arrayfunc.c
+	- assign_array_element_internal: if the assignment failed, as tested by
+	  ASSIGN_DISALLOWED, free the assoc array key we allocated
+	  Report from Grisha Levit <grishalevit@gmail.com>

arrayfunc.c

@@ -276,7 +276,7 @@ bind_array_variable (const char *name, arrayind_t ind, const char *value, int fl
     }
   if (entry == (SHELL_VAR *) 0)
     entry = make_new_array_variable (name);
-  else if ((readonly_p (entry) && (flags&ASS_FORCE) == 0) || noassign_p (entry))
+  else if (ASSIGN_DISALLOWED (entry, flags))
     {
       if (readonly_p (entry))
 	err_readonly (name);
@@ -298,7 +298,7 @@ bind_array_element (SHELL_VAR *entry, arrayind_t ind, char *value, int flags)
 SHELL_VAR *
 bind_assoc_variable (SHELL_VAR *entry, const char *name, char *key, const char *value, int flags)
 {
-  if ((readonly_p (entry) && (flags&ASS_FORCE) == 0) || noassign_p (entry))
+  if (ASSIGN_DISALLOWED (entry, flags))
     {
       if (readonly_p (entry))
 	err_readonly (name);
@@ -406,6 +406,9 @@ assign_array_element_internal (SHELL_VAR *entry, const char *name, char *vname,
       if (estatep)
 	nkey = savestring (akey);	/* assoc_insert/assoc_replace frees akey */
       entry = bind_assoc_variable (entry, vname, akey, value, flags);
+      /* If we didn't perform the assignment, free the key we allocated */
+      if (entry == 0 || (ASSIGN_DISALLOWED (entry, flags)))
+	FREE (akey);
       if (estatep)
 	{
 	  estatep->type = ARRAY_ASSOC;
@@ -476,7 +479,7 @@ find_or_make_array_variable (const char *name, int flags)
 
   if (var == 0)
     var = (flags & 2) ? make_new_assoc_variable (name) : make_new_array_variable (name);
-  else if ((flags & 1) && (readonly_p (var) || noassign_p (var)))
+  else if ((flags & 1) && ASSIGN_DISALLOWED(var, 0))
     {
       if (readonly_p (var))
 	err_readonly (name);

builtins/declare.def

@@ -879,7 +879,7 @@ restart_new_var_name:
 	  NEXT_VARIABLE ();
 	}
       /* Cannot use declare to assign value to readonly or noassign variable. */
-      else if ((readonly_p (var) || noassign_p (var)) && offset)
+      else if (ASSIGN_DISALLOWED (var, 0) && offset)
 	{
 	  if (readonly_p (var))
 	    {

builtins/getopts.def

@@ -117,7 +117,7 @@ getopts_bind_variable (char *name, char *value)
   if (valid_identifier (name))
     {
       v = bind_variable (name, value, 0);
-      if (v && (readonly_p (v) || noassign_p (v)))
+      if (v && ASSIGN_DISALLOWED (v, 0))
 	return (EX_MISCERROR);
       return (v ? EXECUTION_SUCCESS : EXECUTION_FAILURE);
     }

builtins/printf.def

@@ -111,7 +111,7 @@ extern int errno;
 	  SHELL_VAR *v; \
 	  v = builtin_bind_variable  (vname, vbuf, bindflags); \
 	  stupidly_hack_special_variables (vname); \
-	  if (v == 0 || readonly_p (v) || noassign_p (v)) \
+	  if (v == 0 || ASSIGN_DISALLOWED (v, 0)) \
 	    retval = EXECUTION_FAILURE; \
 	  if (vbsize > 4096) \
 	    { \
@@ -334,7 +334,7 @@ printf_builtin (WORD_LIST *list)
       SHELL_VAR *v;
       v = builtin_bind_variable (vname, "", 0);
       stupidly_hack_special_variables (vname);
-      return ((v == 0 || readonly_p (v) || noassign_p (v)) ? EXECUTION_FAILURE : EXECUTION_SUCCESS);
+      return ((v == 0 || ASSIGN_DISALLOWED (v, 0)) ? EXECUTION_FAILURE : EXECUTION_SUCCESS);
     }
 
   /* If the format string is empty after preprocessing, return immediately. */

builtins/read.def

@@ -635,6 +635,8 @@ read_builtin (WORD_LIST *list)
 	  if (fd2 >= 0)
 	    close (fd2);
 	  run_unwind_frame ("read_builtin");
+	  if (free_ifs)
+	    free (ifs_chars);
 	  return (EXECUTION_FAILURE);
 	}
 
@@ -746,6 +748,7 @@ read_builtin (WORD_LIST *list)
 	  x = errno;
 	  if (retval < 0 && errno == EINTR)
 	    {
+	      ZRESET ();
 	      check_signals ();		/* in case we didn't call zread via zreadc */
 	      lastsig = LASTSIG();
 	      if (lastsig == 0)
@@ -764,6 +767,10 @@ read_builtin (WORD_LIST *list)
 	  break;
 	}
 
+      /* Even if read returns a partial buffer, if we got an interrupt we're
+	 going to throw it away. */
+      ZRESET();
+
       QUIT;		/* in case we didn't call check_signals() */
 #if defined (READLINE)
 	}
@@ -894,6 +901,8 @@ add_char:
       if (errno != EINTR)
 	builtin_error ("%d: %s: %s", fd, _("read error"), strerror (errno));
       run_unwind_frame ("read_builtin");
+      if (free_ifs)
+	free (ifs_chars);
       return ((t_errno != EINTR) ? EXECUTION_FAILURE : 128+lastsig);
     }
 
@@ -943,6 +952,8 @@ assign_vars:
       if (var == 0)
 	{
 	  free (input_string);
+	  if (free_ifs)
+	    free (ifs_chars);
 	  return EXECUTION_FAILURE;	/* readonly or noassign */
 	}
 
@@ -957,6 +968,8 @@ assign_vars:
 	  dispose_words (alist);
 	}
       free (input_string);
+      if (free_ifs)
+	free (ifs_chars);
       return (retval);
     }
 #endif /* ARRAY_VARS */ 
@@ -987,12 +1000,14 @@ assign_vars:
 	}
       else
 	var = bind_variable ("REPLY", input_string, 0);
-      if (var == 0 || readonly_p (var) || noassign_p (var))
+      if (var == 0 || ASSIGN_DISALLOWED (var, 0))
 	retval = EX_MISCERROR;
       else
 	VUNSETATTR (var, att_invisible);
 
       free (input_string);
+      if (free_ifs)
+	free (ifs_chars);
       return (retval);
     }
 
@@ -1017,6 +1032,8 @@ assign_vars:
 	{
 	  sh_invalidid (varname);
 	  free (orig_input_string);
+	  if (free_ifs)
+	    free (ifs_chars);
 	  return (EXECUTION_FAILURE);
 	}
 
@@ -1049,6 +1066,8 @@ assign_vars:
       if (var == 0)
 	{
 	  free (orig_input_string);
+	  if (free_ifs)
+	    free (ifs_chars);
 	  return (EX_MISCERROR);
 	}
 
@@ -1066,6 +1085,8 @@ assign_vars:
     {
       sh_invalidid (list->word->word);
       free (orig_input_string);
+      if (free_ifs)
+	free (ifs_chars);
       return (EXECUTION_FAILURE);
     }
 
@@ -1123,8 +1144,7 @@ bind_read_variable (char *name, char *value, int flags)
   SHELL_VAR *v;
 
   v = builtin_bind_variable (name, value, flags);
-  return (v == 0 ? v
-		 : ((readonly_p (v) || noassign_p (v)) ? (SHELL_VAR *)NULL : v));
+  return ((v == 0 || ASSIGN_DISALLOWED (v, 0)) ? (SHELL_VAR *)NULL : v);
 }
 
 #if defined (HANDLE_MULTIBYTE)

config.h.in

@@ -944,6 +944,12 @@
 /* Define if you have the tcgetpgrp function.  */
 #undef HAVE_TCGETPGRP
 
+/* Define if you have the tcgetwinsize function.  */
+#undef HAVE_TCGETWINSIZE
+
+/* Define if you have the tcsetwinsize function.  */
+#undef HAVE_SETWINSIZE
+
 /* Define if you have the times function.  */
 #undef HAVE_TIMES
 

configure

@@ -1,5 +1,5 @@
 #! /bin/sh
-# From configure.ac for Bash 5.3, version 5.064.
+# From configure.ac for Bash 5.3, version 5.065.
 # Guess values for system-dependent variables and create Makefiles.
 # Generated by GNU Autoconf 2.72 for bash 5.3-alpha.
 #
@@ -15757,6 +15757,18 @@ if test "x$ac_cv_func_tcgetattr" = xyes
 then :
   printf "%s\n" "#define HAVE_TCGETATTR 1" >>confdefs.h
 
+fi
+ac_fn_c_check_func "$LINENO" "tcgetwinsize" "ac_cv_func_tcgetwinsize"
+if test "x$ac_cv_func_tcgetwinsize" = xyes
+then :
+  printf "%s\n" "#define HAVE_TCGETWINSIZE 1" >>confdefs.h
+
+fi
+ac_fn_c_check_func "$LINENO" "tcsetwinsize" "ac_cv_func_tcsetwinsize"
+if test "x$ac_cv_func_tcsetwinsize" = xyes
+then :
+  printf "%s\n" "#define HAVE_TCSETWINSIZE 1" >>confdefs.h
+
 fi
 ac_fn_c_check_func "$LINENO" "times" "ac_cv_func_times"
 if test "x$ac_cv_func_times" = xyes

configure.ac

@@ -21,7 +21,7 @@ dnl Process this file with autoconf to produce a configure script.
 #   You should have received a copy of the GNU General Public License
 #   along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-AC_REVISION([for Bash 5.3, version 5.064])dnl
+AC_REVISION([for Bash 5.3, version 5.065])dnl
 
 define(bashvers, 5.3)
 define(relstatus, alpha)
@@ -870,7 +870,8 @@ AC_CHECK_FUNCS(bcopy bzero clock_gettime confstr faccessat fnmatch \
 		getaddrinfo gethostbyname getservbyname getservent inet_aton \
 		imaxdiv memmove pathconf putenv raise random regcomp regexec \
 		setenv setlinebuf setlocale setvbuf siginterrupt strchr \
-		sysconf syslog tcgetattr times ttyname tzset unsetenv)
+		sysconf syslog tcgetattr tcgetwinsize tcsetwinsize \
+		times ttyname tzset unsetenv)
 
 AC_CHECK_FUNCS(vasprintf asprintf)
 AC_CHECK_FUNCS(isascii isblank isgraph isprint isspace isxdigit)

execute_cmd.c

@@ -1453,7 +1453,8 @@ time_command (COMMAND *command, int asynchronous, int pipe_in, int pipe_out, str
     rv = execute_command_internal (command, asynchronous, pipe_in, pipe_out, fds_to_close);
   COPY_PROCENV (save_top_level, top_level);
 
-  command->flags = old_flags;
+  if (code == NOT_JUMPED)
+    command->flags = old_flags;
 
   /* If we're jumping in a different subshell environment than we started,
      don't bother printing timing stats, just keep longjmping back to the
@@ -2366,7 +2367,7 @@ coproc_setvars (struct coproc *cp)
 	}
     }
 
-  if (v && (readonly_p (v) || noassign_p (v)))
+  if (v && ASSIGN_DISALLOWED (v, 0))
     {
       if (readonly_p (v))
 	err_readonly (cp->c_name);
@@ -3040,7 +3041,7 @@ execute_for_command (FOR_COM *for_command)
       else
 	v = bind_variable (identifier, list->word->word, 0);
 
-      if (v == 0 || readonly_p (v) || noassign_p (v))
+      if (v == 0 || ASSIGN_DISALLOWED (v, 0))
 	{
 	  line_number = save_line_number;
 	  if (v && readonly_p (v) && interactive_shell == 0 && posixly_correct)
@@ -3213,6 +3214,21 @@ execute_arith_for_command (ARITH_FOR_COM *arith_for_command)
       expresult = eval_arith_for_expr (arith_for_command->test, &expok);
       line_number = save_lineno;
 
+      /* If the step or test expressions execute `break' or `continue' in a
+	 nofork command substitution or by some other means, break the loop
+	 here. */
+      if (breaking)
+	{
+	  breaking--;
+	  break;
+	}
+      if (continuing)
+	{
+	  continuing--;
+	  if (continuing)
+	    break;
+	}
+
       if (expok == 0)
 	break;
 
@@ -3534,7 +3550,7 @@ execute_select_command (SELECT_COM *select_command)
 	}
 
       v = bind_variable (identifier, selection, 0);
-      if (v == 0 || readonly_p (v) || noassign_p (v))
+      if (v == 0 || ASSIGN_DISALLOWED (v, 0))
 	{
 	  if (v && readonly_p (v) && interactive_shell == 0 && posixly_correct)
 	    {
@@ -6263,7 +6279,7 @@ execute_intern_function (WORD_DESC *name, FUNCTION_DEF *funcdef)
     }
 
   var = find_function (name->word);
-  if (var && (readonly_p (var) || noassign_p (var)))
+  if (var && ASSIGN_DISALLOWED (var, 0))
     {
       if (readonly_p (var))
 	internal_error (_("%s: readonly function"), var->name);

expr.c

@@ -336,7 +336,7 @@ expr_bind_variable (const char *lhs, const char *rhs)
   aflags = 0;
 #endif
   v = bind_int_variable (lhs, rhs, aflags);
-  if (v && (readonly_p (v) || noassign_p (v)))
+  if (v && ASSIGN_DISALLOWED (v, 0))
     sh_longjmp (evalbuf, 1);	/* variable assignment error */
   stupidly_hack_special_variables (lhs);
 }

lib/readline/complete.c

@@ -77,6 +77,10 @@ extern int errno;
 #  include "colors.h"
 #endif
 
+#ifndef MIN
+#define MIN(x,y) (((x) < (y)) ? (x): (y))
+#endif
+
 typedef int QSFUNC (const void *, const void *);
 
 #ifdef HAVE_LSTAT
@@ -1355,6 +1359,7 @@ compute_lcd_of_matches (char **match_list, int matches, const char *text)
   int low;		/* Count of max-matched characters. */
   int lx;
   char *dtext;		/* dequoted TEXT, if needed */
+  size_t si1, si2;
   size_t len1, len2;
 #if defined (HANDLE_MULTIBYTE)
   int v;
@@ -1385,7 +1390,7 @@ compute_lcd_of_matches (char **match_list, int matches, const char *text)
       len1 = strlen (match_list[i]);
       len2 = strlen (match_list[i + 1]);
 
-      for (si = 0; (c1 = match_list[i][si]) && (c2 = match_list[i + 1][si]); si++)
+      for (si1 = si2 = 0; (c1 = match_list[i][si1]) && (c2 = match_list[i + 1][si2]); si1++,si2++)
 	{
 	    if (_rl_completion_case_fold)
 	      {
@@ -1395,8 +1400,8 @@ compute_lcd_of_matches (char **match_list, int matches, const char *text)
 #if defined (HANDLE_MULTIBYTE)
 	    if (MB_CUR_MAX > 1 && rl_byte_oriented == 0)
 	      {
-		v1 = MBRTOWC (&wc1, match_list[i]+si, len1 - si, &ps1);
-		v2 = MBRTOWC (&wc2, match_list[i+1]+si, len2 - si, &ps2);
+		v1 = MBRTOWC (&wc1, match_list[i]+si1, len1 - si1, &ps1);
+		v2 = MBRTOWC (&wc2, match_list[i+1]+si2, len2 - si2, &ps2);
 		if (MB_INVALIDCH (v1) || MB_INVALIDCH (v2))
 		  {
 		    if (c1 != c2)	/* do byte comparison */
@@ -1410,8 +1415,11 @@ compute_lcd_of_matches (char **match_list, int matches, const char *text)
 		  }
 		if (wc1 != wc2)
 		  break;
-		else if (v1 > 1)
-		  si += v1 - 1;
+
+		if (v1 > 1)
+		  si1 += v1 - 1;
+		if (v2 > 1)
+		  si2 += v2 - 1;
 	      }
 	    else
 #endif
@@ -1419,6 +1427,7 @@ compute_lcd_of_matches (char **match_list, int matches, const char *text)
 	      break;
 	}
 
+      si = MIN (si1, si2);	/* use shorter of matches of different length */
       if (low > si)
 	low = si;
     }

lib/readline/text.c

@@ -2011,9 +2011,7 @@ _rl_readstr_init (int pchar, int flags)
 
   cxt = _rl_rscxt_alloc (flags);
 
-  rl_maybe_replace_line ();
   _rl_saved_line_for_readstr = _rl_alloc_saved_line ();
-
   rl_undo_list = 0;
 
   rl_line_buffer[0] = 0;

lib/sh/zread.c

@@ -41,13 +41,16 @@ extern int errno;
 #  define ZBUFSIZ 4096
 #endif
 
-extern int executing_builtin;
+extern int executing_builtin, interrupt_state;
 
 extern void check_signals_and_traps (void);
 extern void check_signals (void);
 extern int signal_is_trapped (int);
 extern int read_builtin_timeout (int);
 
+/* Forward declarations */
+void zreset (void);
+
 /* Read LEN bytes from FD into BUF.  Retry the read on EINTR.  Any other
    error causes the loop to break. */
 ssize_t
@@ -66,7 +69,11 @@ zread (int fd, char *buf, size_t len)
       /* XXX - bash-5.0 */
       /* We check executing_builtin and run traps here for backwards compatibility */
       if (executing_builtin)
-	check_signals_and_traps ();	/* XXX - should it be check_signals()? */
+	{
+	  if (interrupt_state)
+	    zreset ();
+	  check_signals_and_traps ();	/* XXX - should it be check_signals()? */
+	}
       else
 	check_signals ();
       errno = t;

quit.h

@@ -83,4 +83,9 @@ do { \
     } \
 } while (0)
 
+#define ZRESET() \
+  do { \
+    if (interrupt_state) zreset (); \
+  } while (0)
+
 #endif /* _QUIT_H_ */

redir.c

@@ -1417,7 +1417,7 @@ redir_varassign (REDIRECT *redir, int fd)
 
   w = redir->redirector.filename;
   v = bind_var_to_int (w->word, fd, 0);
-  if (v == 0 || readonly_p (v) || noassign_p (v))
+  if (v == 0 || ASSIGN_DISALLOWED (v, 0))
     return BADVAR_REDIRECT;
 
   stupidly_hack_special_variables (w->word);

subst.c

@@ -2295,7 +2295,7 @@ skip_to_delim (const char *string, int start, const char *delims, int flags)
 	    CQ_RETURN(si);
 
 	  if (string[i+1] == LPAREN)
-	    temp = extract_delimited_string (string, &si, "$(", "(", ")", SX_NOALLOC|SX_COMMAND|completeflag); /* ) */
+	    temp = extract_command_subst (string, &si, SX_NOALLOC|SX_COMMAND|completeflag);
 	  else
 	    temp = extract_dollar_brace_string (string, &si, 0, SX_NOALLOC|completeflag);
 	  CHECK_STRING_OVERRUN (i, si, slen, c);
@@ -3450,7 +3450,7 @@ do_compound_assignment (const char *name, char *value, int flags)
     {
       v = find_variable (name);		/* follows namerefs */
       newname = (v == 0) ? nameref_transform_name (name, flags) : v->name;
-      if (v && ((readonly_p (v) && (flags & ASS_FORCE) == 0) || noassign_p (v)))
+      if (v && ASSIGN_DISALLOWED (v, flags))
 	{
 	  if (readonly_p (v))
 	    err_readonly (name);
@@ -3476,7 +3476,7 @@ do_compound_assignment (const char *name, char *value, int flags)
 	v = 0;
       if (v == 0)
         v = find_global_variable (name);
-      if (v && ((readonly_p (v) && (flags & ASS_FORCE) == 0) || noassign_p (v)))
+      if (v && ASSIGN_DISALLOWED (v, flags))
 	{
 	  if (readonly_p (v))
 	    err_readonly (name);
@@ -3502,7 +3502,7 @@ do_compound_assignment (const char *name, char *value, int flags)
   else
     {
       v = assign_array_from_string (name, value, flags);
-      if (v && ((readonly_p (v) && (flags & ASS_FORCE) == 0) || noassign_p (v)))
+      if (v && ASSIGN_DISALLOWED (v, flags))
 	{
 	  if (readonly_p (v))
 	    err_readonly (name);
@@ -8172,7 +8172,7 @@ parameter_brace_expand_rhs (char *name, char *value,
 #endif /* ARRAY_VARS */
   v = bind_variable (vname, t1, 0);
 
-  if (v == 0 || readonly_p (v) || noassign_p (v))	/* expansion error  */
+  if (v == 0 || ASSIGN_DISALLOWED (v, 0))		/* expansion error */
     {
       if ((v == 0 || readonly_p (v)) && interactive_shell == 0 && posixly_correct)
 	{

tests/arith-for.tests

@@ -140,3 +140,7 @@ for ((j=;;)); do :; done
 echo X
 break
 echo Y
+
+# arithmetic for commands need to skip over command substitutions
+for (( $(case x in x) esac);; )); do break; done
+for (( ${ case x in x) esac; };; )); do break; done

variables.c

@@ -3126,7 +3126,7 @@ bind_variable_internal (const char *name, const char *value, HASH_TABLE *table,
     }
   else if (entry->assign_func)	/* array vars have assign functions now */
     {
-      if ((readonly_p (entry) && (aflags & ASS_FORCE) == 0) || noassign_p (entry))
+      if (ASSIGN_DISALLOWED (entry, aflags))
 	{
 	  if (readonly_p (entry))
 	    err_readonly (name_cell (entry));
@@ -3148,7 +3148,7 @@ bind_variable_internal (const char *name, const char *value, HASH_TABLE *table,
   else
     {
 assign_value:
-      if ((readonly_p (entry) && (aflags & ASS_FORCE) == 0) || noassign_p (entry))
+      if (ASSIGN_DISALLOWED (entry, aflags))
 	{
 	  if (readonly_p (entry))
 	    err_readonly (name_cell (entry));
@@ -3581,8 +3581,8 @@ assign_in_env (const WORD_DESC *word, int flags)
 	}
       else
         newname = name_cell (var);	/* no-op if not nameref */
-	  
-      if (var && (readonly_p (var) || noassign_p (var)))
+
+      if (var && ASSIGN_DISALLOWED (var, 0))
 	{
 	  if (readonly_p (var))
 	    err_readonly (name);

variables.h

@@ -238,6 +238,10 @@ typedef struct _vlist {
 /* Special value for nameref with invalid value for creation or assignment */
 extern SHELL_VAR nameref_invalid_value;
 #define INVALID_NAMEREF_VALUE	(void *)&nameref_invalid_value
+
+/* Assignment statements */
+#define ASSIGN_DISALLOWED(v, f) \
+  ((readonly_p (v) && (f&ASS_FORCE) == 0) || noassign_p (v))
 	
 /* Stuff for hacking variables. */
 typedef int sh_var_map_func_t (SHELL_VAR *);