Re: [PATCH]: New hook function "atexec"

[Peter Stephenson <p.w.stephenson@ntlworld.com>]

On Thu, 16 Jul 2009 23:27:49 -0700 (PDT)
Michael Hwang <nomex45@yahoo.com> wrote:
> I've hacked in a new hook function similar to preexec called "atexec".

That's a reasonable idea: some thoughts:

I'm not sure where this needs to be called execcmd(), there's so much
going on there.  The issue I'm most worried about is redirection; where
is the output of hook functions called in the middle of a pipeline going?

However, it also worries me that this is calling execcmd() recursively
in strange ways never before attempted---we partly set up a command for
execution, then call an entire arbitrary execution tree, then finish
executing the command.  This could easily have odd effects.  I wonder if
using execsave() and execrestore() might be sensible.

I'm surprised you only need to set atexec to zero in two places, but
there may be another way of testing the conditions you want anyway.

I don't see why you need the test for sourcelevel, which stops it
happening in all "." files.  That might be sensible for init files, but
otherwise it seems a bit arbitrary when other limitations might be more
useful.  For example, I don't see anything to stop this being run for
every individual command in every function run by the user, which could
be a huge overhead, probably more so that "." files which tend to be
one-off.  (I think the test for "sfcontext" happens to catch a lot of
completion functions, however.)  I'd be tempted to add a "locallevel"
test to exclude all function bodies.  If necessary we can think of a way
of reactivating it in a function.

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/