PATCH: failed autoload

PATCH: failed autoload

[Peter Stephenson]

It can't be correct that if an autoload fails beacause of a syntax error
that the function returns 0.

I don't see how it can be useful to define the function with an empty
function body, either.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.50
diff -u -r1.50 exec.c
--- Src/exec.c	2 May 2003 10:25:32 -0000	1.50
+++ Src/exec.c	1 Jul 2003 18:19:03 -0000
@@ -3339,7 +3339,7 @@
 	return NULL;
     }
     if (!prog)
-	prog = &dummy_eprog;
+	return NULL;
     if (ksh == 2 || (ksh == 1 && isset(KSHAUTOLOAD))) {
 	if (autol) {
 	    prog->flags |= EF_RUN;

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: PATCH: failed autoload

[Bart Schaefer]

On Jul 1,  7:25pm, Peter Stephenson wrote:
}
} It can't be correct that if an autoload fails beacause of a syntax error
} that the function returns 0.
} 
} I don't see how it can be useful to define the function with an empty
} function body, either.
} 
}      if (!prog)
} -	prog = &dummy_eprog;

I'm sure there was a reason for this, at one time.  It was introduced in
zsh-workers/9332 by Sven, with the log message "Use word code instead of
structs for passing executable chunks around."

Re: PATCH: failed autoload

[Peter Stephenson]

Bart Schaefer wrote:
> On Jul 1,  7:25pm, Peter Stephenson wrote:
> } It can't be correct that if an autoload fails beacause of a syntax error
> } that the function returns 0.
> } 
> } I don't see how it can be useful to define the function with an empty
> } function body, either.
>
> I'm sure there was a reason for this, at one time.  It was introduced in
> zsh-workers/9332 by Sven, with the log message "Use word code instead of
> structs for passing executable chunks around."

I'm not sure that's quite the same issue... Before, we would have
returned a NULL in many cases where now we return a pointer to a dummy
eprog.  However, the caller of the function I patched is using NULL to
signal an error (see the `return NULL' just a couple of lines higher).
So it seems to me likely that this got changed incorrectly to flag `no
code present but no error' instead of `failed to load function, error'.

Please contradict if you think my logic is wrong.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: PATCH: failed autoload

[Bart Schaefer]

On Jul 2, 10:55am, Peter Stephenson wrote:
} Subject: Re: PATCH: failed autoload
}
} So it seems to me likely that this got changed incorrectly to flag `no
} code present but no error' instead of `failed to load function, error'.
} 
} Please contradict if you think my logic is wrong.

The difference since the patch mainly seems to be that the autoloaded
function remains undefined (and hence zsh will [attempt to] autoload it
again and again), rather than becoming defined as a no-op function.

Suppose you're calling the failing function in a loop.  Now you'll get
the error repeatedly, as often as the loop manages to keep running,
rather than only once.

schaefer<509> repeat 7 breakthis
breakthis:3: parse error near `\n'
breakthis:3: parse error near `\n'
breakthis:3: parse error near `\n'
breakthis:3: parse error near `\n'
breakthis:3: parse error near `\n'
breakthis:3: parse error near `\n'
breakthis:3: parse error near `\n'

On the other hand, that's what happens even in older zsh with "function
definition file not found" so I'm not sure it's worth preserving the old
behavior.

Re: PATCH: failed autoload

[Peter Stephenson]

Bart Schaefer wrote:
> Suppose you're calling the failing function in a loop.  Now you'll get
> the error repeatedly, as often as the loop manages to keep running,
> rather than only once.
> 
> schaefer<509> repeat 7 breakthis
> breakthis:3: parse error near `\n'
> breakthis:3: parse error near `\n'
>...

But surely that's correct?  If the function gets redefined to a no-op it
silently succeeds, which could in principle cause untold mayhem.

% cat ~/fns/copy_file_to_backup
# deliberate typo
if [[ -f $1 ]]: then
  cp $1 ~/backup_files
fi
% autoload copy_file_to_backup
% for f in *; do copy_file_to_backup $1 && rm -f $1; done

(I haven't dared tried this but I think you get the picture even if the
example's a bit awry.)

I've noticed the old behaviour with trepidation before, but never done
anything about it; I don't remember there being any discussion about
making it a deliberate feature.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************