Re: [PATCH] Skip initial expansion of valid array reference tokens in unset
On Tue, Apr 20, 2021 at 9:35 AM Koichi Murase wrote: > In addition, you need to change `1 << 31' to `1u << 31'. The literal > `1' has the type `(signed) int'. One needs to use `1u'---the literal > of the type `unsigned int'. I see, but I think I'll just skip that as I would also have to modify other declarations for consistency. I think the code is already good enough for a proof of concept at least. -- konsolebox
Re: [PATCH] Skip initial expansion of valid array reference tokens in unset
2021年4月20日(火) 9:14 konsolebox : > On Tue, Apr 20, 2021 at 2:08 AM Koichi Murase wrote: > > AFAIK `1 << 31' causes undefined behavior when int is 32-bit signed > > integer. Instead, I think you may write `(int)(1u << 31)' for > > conforming C compilers. `u << 31' is defined for unsigned integers, > > and the conversion between signed and unsigned integers is defined to > > be made by mod 2^{32} (when int is 32 bits). > > Not sure how this should be done right so I decided to just change > 'flags' to unsigned int. > > - int flags; /* Flags associated with this word. */ > + unsigned int flags; /* Flags associated with this word. */ Yeah, right. I haven't checked the type of `flags'. We also need to make it unsigned if we want to use bit 31. In addition, you need to change `1 << 31' to `1u << 31'. The literal `1' has the type `(signed) int'. One needs to use `1u'---the literal of the type `unsigned int'. -- Koichi
Re: [PATCH] Skip initial expansion of valid array reference tokens in unset
On Tue, Apr 20, 2021 at 2:08 AM Koichi Murase wrote: > AFAIK `1 << 31' causes undefined behavior when int is 32-bit signed > integer. Instead, I think you may write `(int)(1u << 31)' for > conforming C compilers. `u << 31' is defined for unsigned integers, > and the conversion between signed and unsigned integers is defined to > be made by mod 2^{32} (when int is 32 bits). Not sure how this should be done right so I decided to just change 'flags' to unsigned int. - int flags; /* Flags associated with this word. */ + unsigned int flags; /* Flags associated with this word. */ > `y.tab.c' is automatically generated from `parse.y'. You should edit > `parse.y' instead of `y.tab.c' I also modified parse.y. All changes made so far can be seen in https://git.io/JOam0. Thanks for the tips. -- konsolebox
Re: [PATCH] Free unfreed string in assign_assoc_from_kvlist()
On 4/19/21 3:22 PM, Mike Jonkmans wrote: On Mon, Apr 19, 2021 at 11:27:13PM +0800, konsolebox wrote: I looked at this code plenty of times and I really think the returned value to expand_assignment_string_to_string (v, 0) needs to be freed too when aval != 0. ... The patch seems to be correct. Moreover, I think that the line aval = expand_assignment_string_to_string (v, 0); should be moved down below the next if-statement. Because of the continue, there is a memory leak. Good catch. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: Bash Builtin Read Function Hang
On 4/19/21 3:48 PM, Mike Whitton wrote: Bash Version: 5.0 Patch Level: 18 Release Status: release Description: I have a script that I wrote in bash and have been using for years. It worked fine up until I updated bash to the latest version from IBM, 5.0 Patch 18. Previously I ran it without issue on 5.0 with no patches. What is happening is when I run a command (such as 'ls -al /etc') with a while loop and read the line it works fine sometimes but sometimes it does not. When it does not work fine it hangs once it reaches the end of the input that is being fed into the loop, it should see the EOF and automatically terminate but it does not. This previously worked fine 100% of the time. I trussed the output and this is what we are getting when we reach the end of the line: kfcntl(2, F_GETFL, 0x0FE8) = 67110914 kioctl(0, 22528, 0x, 0x) Err#25 ENOTTY lseek(0, 0, 1) Err#29 ESPIPE kread(0, "\t", 1) (sleeping...) When it is working the kread function does not sleep, it just moves onto the next iteration in the while loop. Repeat-By: Here is a sample piece of code to recreate the issue: SGLIST=("testfile1" "testfile1" "testfile1" "testfile1") # List of storage groups for sg in ${SGLIST[@]} do while read line do echo $line done < <(ls -al /etc 2>&1) done This particular sample code reliably fails every time. The piece of code I have written fails with the same basic setup one out of every four times or so. I suspect this has something to do with reaping FIFOs when the writing process exits, but I can't reproduce it on macOS or a couple different Linux versions. Chet -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: [PATCH] Free unfreed string in assign_assoc_from_kvlist()
On 4/19/21 11:27 AM, konsolebox wrote: I looked at this code plenty of times and I really think the returned value to expand_assignment_string_to_string (v, 0) needs to be freed too when aval != 0. Thanks for the report. I think you're right. Chet -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Bash Builtin Read Function Hang
Configuration Information [Automatically generated, do not change]: Machine: powerpc OS: aix6.1.9.0 Compiler: gcc -maix64 Compilation CFLAGS: -g -Wno-parentheses -Wno-format-security uname output: AIX aixutil07 2 7 00C07A304B00 Machine Type: powerpc-ibm-aix6.1.9.0 Bash Version: 5.0 Patch Level: 18 Release Status: release Description: I have a script that I wrote in bash and have been using for years. It worked fine up until I updated bash to the latest version from IBM, 5.0 Patch 18. Previously I ran it without issue on 5.0 with no patches. What is happening is when I run a command (such as 'ls -al /etc') with a while loop and read the line it works fine sometimes but sometimes it does not. When it does not work fine it hangs once it reaches the end of the input that is being fed into the loop, it should see the EOF and automatically terminate but it does not. This previously worked fine 100% of the time. I trussed the output and this is what we are getting when we reach the end of the line: kfcntl(2, F_GETFL, 0x0FE8) = 67110914 kioctl(0, 22528, 0x, 0x) Err#25 ENOTTY lseek(0, 0, 1) Err#29 ESPIPE kread(0, "\t", 1) (sleeping...) When it is working the kread function does not sleep, it just moves onto the next iteration in the while loop. Repeat-By: Here is a sample piece of code to recreate the issue: SGLIST=("testfile1" "testfile1" "testfile1" "testfile1") # List of storage groups for sg in ${SGLIST[@]} do while read line do echo $line done < <(ls -al /etc 2>&1) done This particular sample code reliably fails every time. The piece of code I have written fails with the same basic setup one out of every four times or so. Thanks
Re: [PATCH] Free unfreed string in assign_assoc_from_kvlist()
On Mon, Apr 19, 2021 at 11:27:13PM +0800, konsolebox wrote: > I looked at this code plenty of times and I really think the returned > value to expand_assignment_string_to_string (v, 0) needs to be freed > too when aval != 0. > > ... The patch seems to be correct. Moreover, I think that the line aval = expand_assignment_string_to_string (v, 0); should be moved down below the next if-statement. Because of the continue, there is a memory leak. Regards, Mike Jonkmans
Re: [PATCH] Skip initial expansion of valid array reference tokens in unset
2021年4月20日(火) 0:16 konsolebox : > Attached patch demonstrates a solution that solves the current issues > of unset discussed in > https://lists.gnu.org/archive/html/bug-bash/2021-03/msg00056.html > while avoiding breakage of scripts and keeping the expansion of > subscripts consistent with expansion in references. Nice demonstration! I haven't tried but have comments after taking a glance at the patch. > diff --git a/command.h b/command.h > index 914198f..4dafcf0 100644 > --- a/command.h > +++ b/command.h > @@ -104,6 +104,7 @@ enum command_type { cm_for, cm_case, cm_while, cm_if, > cm_simple, cm_select, > #define W_CHKLOCAL (1 << 28) /* check for local vars on assignment */ > #define W_NOASSNTILDE (1 << 29) /* don't do tilde expansion like an > assignment statement */ > #define W_FORCELOCAL (1 << 30) /* force assignments to be to local > variables, non-fatal on assignment errors */ > +#define W_NOEXPAND (1 << 31) /* inhibits any form of expansion */ AFAIK `1 << 31' causes undefined behavior when int is 32-bit signed integer. Instead, I think you may write `(int)(1u << 31)' for conforming C compilers. `u << 31' is defined for unsigned integers, and the conversion between signed and unsigned integers is defined to be made by mod 2^{32} (when int is 32 bits). > diff --git a/y.tab.c b/y.tab.c > index dcc5b7f..a9ae0e6 100644 > --- a/y.tab.c > +++ b/y.tab.c > @@ -7700,7 +7700,15 @@ got_token: `y.tab.c' is automatically generated from `parse.y'. You should edit `parse.y' instead of `y.tab.c'. The corresponding lines are parse.y:5512--5517. To regenerate `y.tab.c' after the edit, maybe you need bison (or yacc). -- Koichi
Re: enhancement merge request
> On Apr 19, 2021, at 7:18 AM, Greg Wooledge wrote: > > On Mon, Apr 19, 2021 at 01:16:48AM -0400, Grisha Levit wrote: >> I think you can do something similar to the patch directly in the shell by >> temporarily unsetting HISTFILE and then deleting history entries in a >> certain range and restoring HISTFILE when you are ready > > I still think my solution fits the criteria exactly. There was *no* > stated requirement that the "private mode" could not be a separate > shell process. This wasn't explicitly stated either, but it seems like Ananth wants "private mode" to have access to preceding history. Perhaps something resembling this (with `history -a` instead, if preferred)? private_mode() { history -w bash --rcfile <( echo source ~/.bashrc echo history -r echo unset HISTFILE ) } -- vq
[PATCH] Free unfreed string in assign_assoc_from_kvlist()
I looked at this code plenty of times and I really think the returned value to expand_assignment_string_to_string (v, 0) needs to be freed too when aval != 0. diff --git a/arrayfunc.c b/arrayfunc.c index 8231ba1e..48c43294 100644 --- a/arrayfunc.c +++ b/arrayfunc.c @@ -568,8 +568,6 @@ assign_assoc_from_kvlist (var, nlist, h, flags) for (list = nlist; list; list = list->next) { - free_aval = 0; - k = list->word->word; v = list->next ? list->next->word->word : 0; @@ -589,12 +587,10 @@ assign_assoc_from_kvlist (var, nlist, h, flags) { aval = (char *)xmalloc (1); aval[0] = '\0'; /* like do_assignment_internal */ - free_aval = 1; } bind_assoc_var_internal (var, h, akey, aval, flags); - if (free_aval) - free (aval); + free (aval); } } -- konsolebox
[PATCH] Skip initial expansion of valid array reference tokens in unset
Attached patch demonstrates a solution that solves the current issues of unset discussed in https://lists.gnu.org/archive/html/bug-bash/2021-03/msg00056.html while avoiding breakage of scripts and keeping the expansion of subscripts consistent with expansion in references. The patched code is also available in https://github.com/konsolebox/bash/tree/skip_expansion_of_valid_array_refs_in_unset. The patch is applied against the latest commit (f3a35a2d60) in master branch. The changes can be conveniently compared in https://git.io/JOgrE. The solution allows the following tests to pass. A few of them were described to fail in the other thread. - #!/bin/bash declare -A a key='$(echo foo)' # Here the tokens are valid array references and are not expanded. # The references are completely similar to the assignments. # This solves the surprise expansion issues. a[$key]=1 unset a[$key] declare -p a # Displays no element a['$key']=2 unset a['$key'] declare -p a # Displays no element a["foo"]=3 unset a["foo"] declare -p a # Displays no element echo - # Here the tokens are "strings". They expand and keep the # original behavior and allows existing scripts to not break. # It also allows nref or iref references to be transparently # referenced in it. a[$key]=1 unset 'a[$key]' # Transforms to a[$key] after expansion declare -p a # Displays no element a['$key']=2 unset "a['\$key']" # Transforms to a['$key'] after expansion declare -p a # Displays no element a["foo"]=3 unset 'a["foo"]' # Transforms to a["foo"] after expansion declare -p a # Displays no element echo - # The update also keeps compatibility with already existing behavior of # unset when assoc_expand_once is enabled, but only for quoted tokens. # I would prefer it totally removed though. # Also notice assoc_expand_once's limitation below. shopt -s assoc_expand_once a[$key]=1 unset "a[$key]" declare -p a # Displays no element a['$key']=2 unset "a[\$key]" declare -p a # Displays no element a["foo"]=3 unset "a[foo]" declare -p a # Displays no element echo -- # For unsetting '@' and all elements: key=@ declare -A a=(@ v0 . v1) unset a[$key] declare -p a # Displays 'declare -A a=([.]="v1" )' declare -A a=(@ v0 . v1) unset a[@] declare -p a # Displays 'declare: a: not found' echo - shopt -u assoc_expand_once declare -A a=(@ v0 . v1) unset 'a[$key]' declare -p a # Displays 'declare -A a=([.]="v1" )' declare -A a=(@ v0 . v1) unset 'a[@]' declare -p a # Displays 'declare: a: not found' echo - shopt -s assoc_expand_once declare -A a=(@ v0 . v1) unset "a[$key]" declare -p a # Displays 'declare: a: not found' (A limitation) declare -A a=(@ v0 . v1) unset 'a[@]' declare -p a # Displays 'declare: a: not found' -- konsolebox diff --git a/builtins/set.def b/builtins/set.def index 8ee0165..6ef1d17 100644 --- a/builtins/set.def +++ b/builtins/set.def @@ -872,10 +872,6 @@ unset_builtin (list) else if (unset_function && nameref) nameref = 0; -#if defined (ARRAY_VARS) - vflags = assoc_expand_once ? (VA_NOEXPAND|VA_ONEWORD) : 0; -#endif - while (list) { SHELL_VAR *var; @@ -890,6 +886,9 @@ unset_builtin (list) unset_variable = global_unset_var; #if defined (ARRAY_VARS) + vflags = (assoc_expand_once && (list->word->flags & W_NOEXPAND) == 0) ? + (VA_NOEXPAND|VA_ONEWORD) : 0; + unset_array = 0; /* XXX valid array reference second arg was 0 */ if (!unset_function && nameref == 0 && valid_array_reference (name, vflags)) diff --git a/command.h b/command.h index 914198f..4dafcf0 100644 --- a/command.h +++ b/command.h @@ -104,6 +104,7 @@ enum command_type { cm_for, cm_case, cm_while, cm_if, cm_simple, cm_select, #define W_CHKLOCAL (1 << 28) /* check for local vars on assignment */ #define W_NOASSNTILDE (1 << 29) /* don't do tilde expansion like an assignment statement */ #define W_FORCELOCAL (1 << 30) /* force assignments to be to local variables, non-fatal on assignment errors */ +#define W_NOEXPAND (1 << 31) /* inhibits any form of expansion */ /* Flags for the `pflags' argument to param_expand() and various parameter_brace_expand_xxx functions; also used for string_list_dollar_at */ diff --git a/parser.h b/parser.h index 59bddac..a70481a 100644 --- a/parser.h +++ b/parser.h @@ -48,6 +48,7 @@ #define PST_REDIRLIST 0x08 /* parsing a list of redirections preceding a simple command name */ #define PST_COMMENT 0x10 /* parsing a shell comment; used by aliases */ #define PST_ENDALIAS 0x20 /* just finished expanding and consuming an alias */ +#define PST_UNSET 0x40 /* Definition of the delimiter stack. Needed by parse.y and bashhist.c. */ struct dstack { diff --git a/subst.c b/subst.c index 6132316..da391d2 100644 --- a/subst.c +++ b/subst.c @@ -4378,6 +4378,8 @@ dequote_list (list) for (tlist = list; tlist; tlist = tlist->next) { + if (tlist->word->flags & W_NOEXPAND) + continue; s =
Re: enhancement merge request
On 4/18/21 7:12 PM, Ananth Chellappa wrote: Chet, Lawrence, Sincerely appreciate your time. Far as I understand, there is no way to accomplish what I want - concisely : *get a true private mode* (no logging to HISTFILE *OR* recall with history command after exiting private-mode (toggle of history using set -/+ o) *without sacrificing productivity*. That is, when you are *in private mode, you DO WANT recall with !number and arrow keys. OK, so there are two requirements so far, which can be boiled down to one: 1. Commands continue to be saved in the history list during `private mode' and are deleted from the history list when exiting private mode. The easiest way to do this is to save the history list to a file when entering private mode and restore it from the file when exiting. You can do this with a pair of shell functions and an exit trap, depending on how elaborate you want to be. It's no more of a burden than setting and unsetting a new option to enable and disable this new behavior. It's still incompletely specified. For instance, what happens when you run `history -w' while in `private mode'? Or read new entries from a file using `history -r'? -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: Document variable names need to be all ASCII
On 4/19/21 8:28 AM, 積丹尼 Dan Jacobson wrote: $ e哈=1 bash: e哈=1: command not found OK, but on man bash is it ever mentioned that a variable name must be all ASCII? name A word consisting only of alphanumeric characters and under- scores, and beginning with an alphabetic character or an under- score. Also referred to as an identifier. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: Document variable names need to be all ASCII
On Mon, Apr 19, 2021 at 08:28:54PM +0800, 積丹尼 Dan Jacobson wrote: > $ e哈=1 > bash: e哈=1: command not found > OK, but on man bash is it ever mentioned that a variable name must be all > ASCII? > > ENVIRONMENT >When a program is invoked it is given an array of strings called the >environment. This is a list of name-value pairs, of the form >name=value > > PARAMETERS >A parameter is an entity that stores values. It can be a name, a num‐ >ber, or one of the special characters listed below under Special Param‐ >eters. A variable is a parameter denoted by a name. A variable has > ... DEFINITIONS name A word consisting only of alphanumeric characters and under‐ scores, and beginning with an alphabetic character or an under‐ score. Also referred to as an identifier. "Alphanumeric" here clearly means [[:alnum:]] within the POSIX locale.
Re: Document variable names need to be all ASCII
What a 'name' is, is further defined under "Definitions": "name: A word consisting solely of letters, numbers, and underscores, ..." But it seems you're right that it doesn't say the locale's idea of letters isn't taken into account. Some other shells do accept those. On Mon, Apr 19, 2021 at 4:28 PM 積丹尼 Dan Jacobson wrote: > $ e哈=1 > bash: e哈=1: command not found > OK, but on man bash is it ever mentioned that a variable name must be all > ASCII? > > ENVIRONMENT >When a program is invoked it is given an array of strings called > the >environment. This is a list of name-value pairs, of the > form >name=value > > PARAMETERS >A parameter is an entity that stores values. It can be a name, a > num‐ >ber, or one of the special characters listed below under Special > Param‐ >eters. A variable is a parameter denoted by a name. A variable > has ... > >
Document variable names need to be all ASCII
$ e哈=1 bash: e哈=1: command not found OK, but on man bash is it ever mentioned that a variable name must be all ASCII? ENVIRONMENT When a program is invoked it is given an array of strings called the environment. This is a list of name-value pairs, of the form name=value PARAMETERS A parameter is an entity that stores values. It can be a name, a num‐ ber, or one of the special characters listed below under Special Param‐ eters. A variable is a parameter denoted by a name. A variable has ...
Re: enhancement merge request
On Mon, Apr 19, 2021 at 01:16:48AM -0400, Grisha Levit wrote: > I think you can do something similar to the patch directly in the shell by > temporarily unsetting HISTFILE and then deleting history entries in a > certain range and restoring HISTFILE when you are ready I still think my solution fits the criteria exactly. There was *no* stated requirement that the "private mode" could not be a separate shell process. In fact, running a separate shell as your "private mode" shell makes perfect sense to me. It's more "secure" that way, right? All the crazy backflips that are being attempted in order to make the "private mode" coexist within the same shell process as "regular mode" can simply go away once you decide to separate the two.