bug-bash
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

shopt compat


From: Martin Kealey
Subject: shopt compat
Date: Wed, 14 Aug 2024 18:25:26 +1200 (NZST)
User-agent: Alpine 2.21 (DEB 202 2017-01-01)

Hi Chet

I have worked up a patch that considerably simplifies the logic for
setting and displaying the shopt compatXX settings, by getting rid of
the numberous boolean variables and simply computing the setting level
directly.

However before I submit my compatXX patch, I would like to ensure I've
correctly accounted for the intention of the commit below, which I
assume is fairly described by this comment:

> If we're unsetting one of the compatibility options, make sure the
> current value is in the range of the compatNN space.

In partiular, I'm puzzled by the introduction of this boolean expression

> (oldval > 44 && shell_compatibility_level < DEFAULT_COMPAT_LEVEL)

as it does not seem to reflect the purpose expressed above. If I've
traced the logic correctly, it's blocking "shopt -u compatNN"  when the
current level in within a certain range, and allowing it otherwise.
(Moreover, that range was empty when the commit was made.)

For example, once it's possible to use 'shopt -s compat52', we would then
find that 'shopt -u compat52' would have no effect, unless the number
"44" is updated to match any new available levels.

What was the original purpose of this? Was it just to double-check that
there weren't bugs in the rest of the shell_compatibility_level logic?
If so, would it be acceptable to omit this check from my new version?

-Martin

PS: This patch is dependent on a rather larger patch that will unify the
list of options that can be controled by shopt and set -o, while also
reducing code duplication.  Before I submit that patch, is there any
guidance for formatting, naming, C standard version, etc?

> commit 6d69b625471182f226993aa977c9f4c49d9dbba1
> Author: Chet Ramey <chet.ramey@case.edu>
> Date:   2022-02-15 11:52:30 -0500
>
>  changes for the shopt compatNN options
>
>
> diff --git a/builtins/shopt.def b/builtins/shopt.def
> index 52204c7..33d61d4 100644
> --- a/builtins/shopt.def
> +++ b/builtins/shopt.def
> @@ -646,9 +646,14 @@ set_compatibility_level (option_name, mode)
>       char *option_name;
>       int mode;
>  {
> -  int ind;
> +  int ind, oldval;
>    char *rhs;
>
> +  /* If we're unsetting one of the compatibility options, make sure the
> +     current value is in the range of the compatNN space. */
> +  if (mode == 0)
> +    oldval = shell_compatibility_level;
> +
>    /* If we're setting something, redo some of the work we did above in
>       toggle_shopt().  Unset everything and reset the appropriate option
>       based on OPTION_NAME. */
> @@ -676,6 +681,8 @@ set_compatibility_level (option_name, mode)
>      shell_compatibility_level = 43;
>    else if (shopt_compat44)
>      shell_compatibility_level = 44;
> +  else if (oldval > 44 && shell_compatibility_level < DEFAULT_COMPAT_LEVEL)
> +    ;
>    else
>      shell_compatibility_level = DEFAULT_COMPAT_LEVEL;
>
> @@ -698,6 +705,8 @@ set_compatibility_opts ()
>    switch (shell_compatibility_level)
>      {
>        case DEFAULT_COMPAT_LEVEL:
> +      case 51:                 /* completeness */
> +      case 50:
>         break;
>        case 44:
>         shopt_compat44 = 1; break;



reply via email to

[Prev in Thread] Current Thread [Next in Thread]