Re: [PATCH] Improved LINENO support

[Harald van Dijk <harald@gigawatt.nl>]

On 10/11/10 22:59, Jilles Tjoelker wrote:
> What I was thinking of was adding a lineno field to narg instead of to
> all command types. The lineno variable would then be set by expand.c. I
> think that leads to a smaller patch and it should still give a sensible
> value for almost all errors.  Downside is a few more int-to-ascii
> conversions. A few divisions is not that bad relative to other things
> that are done but printf in modern libcs is bloated and slow and might
> slow down things measurably.

I made an attempt at this, but managed to keep the printing in one place 
by making LINENO a special variable, which is stored in an int and set 
in expandarg. When $LINENO is evaluated, the code checks to see if the 
variable is LINENO, and if so, converts the value to a string. This is 
one simple pointer comparison in the common case in lookupvar:
   if (v == &vlineno && v->text == linenovar)
and avoids sprintf in scripts that don't use LINENO.

> FreeBSD's code subtracts the function's starting line number in the
> parser rather than at run time. I wonder which is best. (FreeBSD
> neglects to save and restore the value, so nested function definitions
> may lead to incorrect line numbers, but that could be fixed.)

I used this approach in my updated patch, by resetting plinno to 0 in 
simplecmd, and restoring it after calling command().

> I think this patch will increase dash's code size at least a little,
> which is always a consideration here.

The code size varies little: the current limited LINENO support gives an 
executable size of 67680 bytes (default configure settings, no libedit, 
gcc 4.5.1, CFLAGS=-Os, strip after compilation), my previous patch gives 
67688 bytes, my latest patch gives 67704 bytes. I do not think 24 bytes 
should be a problem.

> I can also look at getting this
> into FreeBSD sh.

That would be great.

I will send the updated patch when I can properly test it. Basic testing 
shows it works, but more extensive testing fails. As it turns out, this 
is unrelated to my changes: current git, unmodified, has a problem:

$ cat test.sh
echo
cat <<_ACEOF
$0 $@
_ACEOF
echo
$ src/dash test.sh wtf

test.sh wtf
�����
$

I had not noticed it with my earlier patch, because I actually did my 
testing with modified 0.5.6.1 sources.

Thanks for the comments.

Cheers,
Harald