Re: avoid mktemp/mkstemp internally

2016-05-16 Thread Chet Ramey 
On 5/13/16 2:40 PM, Mike Frysinger wrote:
> i was pointed at a bug report for FreeBSD systems [1] where running lots
> of processes in parallel would randomly fail with errors like:
>   cannot make pipe for process substitution: File exists
> 
> upstream FreeBSD addressed this by defining USE_MKTEMP [2] & USE_MKSTEMP
> [3] when building bash.  looking at the source in bash though, i can't
> see why these aren't always defined.  why does bash try to reimplement
> both funcs ad-hoc instead of just using the stable/guaranteed system
> versions ?

Because many traditional implementations of mktemp/mkstemp suck.

> the bash versions seem like it's pretty trivial to collide: it mixes
> current seconds count, current pid number, and a counter. 

Not quite; the calls in bash mix in the return value from the system's
random().  Now, if that sucks too, you're going to lose.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/



signature.asc
Description: OpenPGP digital signature

Re: avoid mktemp/mkstemp internally

2016-05-16 Thread Mike Frysinger 
On 16 May 2016 10:02, Chet Ramey wrote:
> On 5/13/16 2:40 PM, Mike Frysinger wrote:
> > i was pointed at a bug report for FreeBSD systems [1] where running lots
> > of processes in parallel would randomly fail with errors like:
> >   cannot make pipe for process substitution: File exists
> > 
> > upstream FreeBSD addressed this by defining USE_MKTEMP [2] & USE_MKSTEMP
> > [3] when building bash.  looking at the source in bash though, i can't
> > see why these aren't always defined.  why does bash try to reimplement
> > both funcs ad-hoc instead of just using the stable/guaranteed system
> > versions ?
> 
> Because many traditional implementations of mktemp/mkstemp suck.

and many modern implementations work perfectly fine.  why is the default
to penalize good/fixed versions ?  how about we flip this in config-top.h
like the attached patch ?

> > the bash versions seem like it's pretty trivial to collide: it mixes
> > current seconds count, current pid number, and a counter. 
> 
> Not quite; the calls in bash mix in the return value from the system's
> random().  Now, if that sucks too, you're going to lose.

except bash isn't calling srand anywhere that i can see, so you're
iterating over the same values every time.  i'm not sure how this
implementation wouldn't also fall into the "inferior" bucket.
-mike

--- a/config-top.h
+++ b/config-top.h
@@ -152,3 +152,9 @@
 /* Define to the maximum level of recursion you want for the source/. builtin.
0 means the limit is not active. */
 #define SOURCENEST_MAX 0
+
+/* Define if your mktemp implementation is sane.  */
+#define USE_MKTEMP
+
+/* Define if your mkstemp implementation is sane.  */
+#define USE_MKTEMP


signature.asc
Description: Digital signature

Re: param expansion with single-character special vars in the environment

2016-05-16 Thread Chet Ramey 
On 5/14/16 12:41 PM, Grisha Levit wrote:
> On Mon, May 2, 2016 at 2:30 PM, Chet Ramey  > wrote:
> 
> I ended up writing a new function: valid_nameref_value(name, flags) which
> I can customize to serve this purpose.
> 
> Looks like this ends up getting bypassed in some cases when the value is an
> empty string:

Yes, it was.  That was intended to solve a specific problem, so I solved
it in a different way.  Thanks for the additional test cases.

Chet

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Re: param expansion with single-character special vars in the environment

2016-05-16 Thread Chet Ramey 
On 5/14/16 12:16 PM, Grisha Levit wrote:
> One more case, which currently segfaults:
> 
> |declare -n REPLY; read <<< / |
> 
> easy fix:

Thanks for the fix.

Chet

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Re: nameref in temp environment

2016-05-16 Thread Chet Ramey 
On 5/12/16 8:58 PM, Grisha Levit wrote:
> The 20160506 snapshot changes assign_in_env to do some nameref-related
> checks on the variable name, but these don’t really seem to make sense
> since assignments in the temporary env just create regular variables and
> don’t follow the nameref chain otherwise.
> 
> i.e. bash doesn’t actually care that |ref| is a nameref at the outer scope
> for the purposes of creating the temp env:

It should, for compatibility; ksh93 follows the nameref at least for
temporary assignments preceding builtins.  I'll look further when I have
a block of time.

Chet
-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Re: coproc and existing variables

2016-05-16 Thread Grisha Levit 
A couple of minor things after the change (i don’t know if they’re worth
looking at..)

   - If coproc name is a nameref to an unset variable, a nameref/array is
   left over:

$ declare -n ref=x; coproc ref { :; }; wait; declare -p ref
[1] 13267
[1]+  Donecoproc ref { :; }declare -an ref=([0]="x")


   - check_unbind_variable calls builtin_error but the unbind-ing happens
   not while coproc is executing. maybe internal_error would make more sense?
   - The %s_PID variable is unbound unconditionally

$ declare -r RO RO_PID; coproc RO { :; }; wait; declare -p RO RO_PID
bash: RO: readonly variable
[1] 13288
[1]+  Donecoproc RO { :; }
bash: wait: RO: cannot unset: readonly variable
declare -r RO
bash: declare: RO_PID: not found

​

On Sun, Apr 24, 2016 at 2:17 PM, Chet Ramey  wrote:

> On 4/21/16 2:39 PM, Grisha Levit wrote:
> > On Thu, Apr 21, 2016 at 1:22 PM, Chet Ramey  wrote:
> >>
> >>> 1. coproc unsets readonly NAME after the process completes
> >>
> >> Yes.  This is a gray area.  Under some circumstances, e.g, getopts with
> >> OPTARG, defined shell behavior can override a readonly setting.
> >
> > Probably should at least disallow this case?
> >
> > $ bash -r -c 'coproc PATH { :; }; wait; PATH=/whatever; echo $PATH'
> > bash: PATH: readonly variable
> > bash: PATH: readonly variable
> > /whatever
>
> Yes, even though the restricted shell is not a great example of anything,
> it seems reasonable to follow printf/read/mapfile and not overwrite read-
> only variables used as coproc names.  getopts will remain an outlier.
>
> Chet
> --
> ``The lyf so short, the craft so long to lerne.'' - Chaucer
>  ``Ars longa, vita brevis'' - Hippocrates
> Chet Ramey, ITS, CWRUc...@case.edu
> http://cnswww.cns.cwru.edu/~chet/
>

jobs race condition

2016-05-16 Thread Grisha Levit 
I would expect the following loop to exit after one (or maybe two)
iterations but it runs until PIDs get recycled:

$ : & while [[ $(jobs) ]]; do :; done
[1] 61589
TRACE: pid 51259: delete_old_job: found pid 61589 in job 0 with state 4

The loop does not terminate because jobs just keeps printing [1]+ Done :.

Hitting ^C during this loop sometimes puts the shell in a really weird
state, with half the output echoed back and half seemingly going to
readline but not echoed back. Sometimes bash: wait_for: No record of
process x is printed.
​

Re: jobs race condition

2016-05-16 Thread Chet Ramey 
On 5/16/16 7:16 PM, Grisha Levit wrote:
> I would expect the following loop to exit after one (or maybe two)
> iterations but it runs until PIDs get recycled:
> 
> |$ : & while [[ $(jobs) ]]; do :; done [1] 61589 TRACE: pid 51259:
> delete_old_job: found pid 61589 in job 0 with state 4 |
> 
> The loop does not terminate because |jobs| just keeps printing |[1]+ Done :|.

It's not a race condition.  Since you're running an interactive shell,
jobs don't get removed from the jobs table until the user is notified
by the interactive shell.  That never happens, since it happens before
the next prompt is printed.

The subshell started to run the command substitution inherits the jobs
list (special cased; so you can do joblist=$(jobs)), so all the command
substitutions get the same list.

Chet

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/

Re: jobs race condition

2016-05-16 Thread Grisha Levit 
On Mon, May 16, 2016 at 8:50 PM, Chet Ramey  wrote:

> jobs don't get removed from the jobs table until the user is notified
> by the interactive shell.


Thanks, that makes perfect sense.  I'll make a new thread with a better
description of the other ^C issue.  It's actually not related to `jobs' at
all..