On Mon, Aug 30, 2021 at 8:55 AM Roberto A. Foglietta
<[email protected]> wrote:
> Hi Denis,
>
> supported by the experience of Harald, I developed this patch far enough to
> be ready for integration. These are the features that will add to busybox ash:
>
> - trap ERR and set -E added
> - global FUNCNAME added
> - LINENO became global
Can this be reasonably split up, or it is too interrelated?
E.g. add FUNCNAME in a separate patch?
Add set -E in a separate patch?
> The patch is in attachment with its test suite.
Can you git send-email the patches?
#define linenovar (G_var.linenovar )
+#define funcnamevar (G_var.funcnamevar )
+#define funcname (G_var.funcname )
+#define doingtrap (G_var.doingtrap )
Inconsistent whitespace
+ lineno = 0; \
strcpy(linenovar, "LINENO="); \
vlineno.var_text = linenovar; \
+ strcpy(funcnamevar, "FUNCNAME="); \
+ vfuncname.var_text = funcnamevar; \
+ funcname = NULL; \
+ doingtrap = 0; \
Zero initializers are not needed, this area is xzalloc()'ed.
+ fmtstr(funcnamevar+9,
sizeof(funcnamevar)-9, "%s",
+ funcname ? funcname : "");
strncpy?
- for (tp = trap; tp < &trap[NSIG]; tp++) {
+ for (tp = trap; tp < &trap[NSIG+1]; tp++) { /* increased by 1:
trap ERR handler */
Let's use better constants here. Something like
"tp <= &trap[NTRAP_LAST]" would be more readable
than the comment.
- if (checkexit & status)
- raise_exception(EXEND);
+ if (checkexit & status) {
+ if (trap[NSIG]) {
Maybe make it look like "if (trap[NTRAP_ERR])" ?
+ static bool recursive = 0, savetrap;
You have one extra tab level here.
+ static int savelineno;
+ if (!recursive) {
+ int err;
+ struct jmploc *volatile
savehandler = exception_handler;
+ struct jmploc jmploc;
+
+ err = setjmp(jmploc.loc);
+ if (!err) {
+ exception_handler = &jmploc;
+<-----><------><------><------><------>
Trailing whitespace
+ savestatus = exitstatus;
+ savetrap = doingtrap;
+ savelineno = lineno;
+ doingtrap = 1;
+ recursive = 1;
+
+ evalstring(trap[NSIG], 0);
+ }
+
+ exception_handler = savehandler;
+ if (err && exception_type != EXERROR)
+
longjmp(exception_handler->loc, 1);
+<-----><------><------><------><------>
+ recursive = 0;
+ lineno = savelineno;
+ doingtrap = savetrap;
What does doingtrap do? Inhibits line counting?
In this case, you don't need to save/restore lineno?
It seems you can replace the above save/restore of doingtrap
if you change it from bool to counter?
"static bool recursive" functionality can be achieved if you
temporarily set trap[NTRAP_ERR] to NULL while running it.
+ funcname = strdup(func->n.ndefun.text);
xstrdup
+ if ((exitstatus && eflag) || (e)) {
+ /* we are exiting within the function */
+ if (savefuncname)
+ free(savefuncname);
+ if (savetrap)
+ free(savetrap);
If we are exiting, why bother free()ing?
+ } else {
+ if (funcname)
+ free(funcname);
free(NULL) works, so don't check for NULL.
A wider concern is that dash development has become reasonably active
and we need to backport fixes from dash. This means that divergent
development in our code creates future problems.
Please ask on dash mailing list whether they are willing to take
patches implementing trap "ERR".
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
