Re: [PATCH] Improved LINENO support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]     [Photo]

  Powered by Linux