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
[LARTC] [Bugtraq] [Yosemite Forum] [Photo]