opt: Warn about ignored expressions after dangerous actions

For example, `bfs -delete -type f` is almost certainly a mistake.

Link: https://savannah.gnu.org/bugs/?65895

3 files changed, 79 insertions, 25 deletions

diff --git a/src/ctx.h b/src/ctx.h
index 064e8b0..c7ebc20 100644
--- a/src/ctx.h
+++ b/src/ctx.h
@@ -68,11 +68,16 @@ struct bfs_ctx {
 	bool status;
 	/** Whether to only return unique files (-unique). */
 	bool unique;
-	/** Whether to print warnings (-warn/-nowarn). */
-	bool warn;
 	/** Whether to only handle paths with xargs-safe characters (-X). */
 	bool xargs_safe;
 
+	/** Whether bfs was run interactively. */
+	bool interactive;
+	/** Whether to print warnings (-warn/-nowarn). */
+	bool warn;
+	/** Whether any dangerous actions (-delete/-exec) are present. */
+	bool dangerous;
+
 	/** Color data. */
 	struct colors *colors;
 	/** The error that occurred parsing the color table, if any. */
diff --git a/src/opt.c b/src/opt.c
index f735924..2609c77 100644
--- a/src/opt.c
+++ b/src/opt.c
@@ -615,21 +615,25 @@ static bool is_const(const struct bfs_expr *expr) {
 
 /** Warn about an expression. */
 _printf(3, 4)
-static void opt_warning(const struct bfs_opt *opt, const struct bfs_expr *expr, const char *format, ...) {
+static bool opt_warning(const struct bfs_opt *opt, const struct bfs_expr *expr, const char *format, ...) {
 	if (!opt->warn) {
-		return;
+		return false;
 	}
 
 	if (bfs_expr_is_parent(expr) || is_const(expr)) {
-		return;
+		return false;
 	}
 
-	if (bfs_expr_warning(opt->ctx, expr)) {
-		va_list args;
-		va_start(args, format);
-		bfs_vwarning(opt->ctx, format, args);
-		va_end(args);
+	if (!bfs_expr_warning(opt->ctx, expr)) {
+		return false;
 	}
+
+	va_list args;
+	va_start(args, format);
+	bfs_vwarning(opt->ctx, format, args);
+	va_end(args);
+
+	return true;
 }
 
 /** Remove and return an expression's children. */
@@ -1807,11 +1811,38 @@ static struct bfs_expr *data_flow_leave(struct bfs_opt *opt, struct bfs_expr *ex
 	return visit_leave(opt, expr, visitor);
 }
 
+/**
+ * Warn about an ignored expression.
+ *
+ * @return
+ *         True to continue optimizing, false to cancel.
+ */
+static bool opt_ignore(struct bfs_opt *opt, struct bfs_expr *expr) {
+	struct bfs_ctx *ctx = opt->ctx;
+
+	if (opt_warning(opt, expr, "The result of this expression is ignored.\n")) {
+		if (ctx->interactive && ctx->dangerous) {
+			bfs_warning(ctx, "Do you want to continue? ");
+			if (ynprompt() == 0) {
+				errno = 0;
+				return false;
+			}
+		}
+
+		fprintf(stderr, "\n");
+	}
+
+	return true;
+}
+
 /** Data flow visitor function. */
 static struct bfs_expr *data_flow_visit(struct bfs_opt *opt, struct bfs_expr *expr, const struct visitor *visitor) {
 	if (opt->ignore_result && expr->pure) {
 		opt_debug(opt, "ignored result\n");
-		opt_warning(opt, expr, "The result of this expression is ignored.\n\n");
+		if (!opt_ignore(opt, expr)) {
+			return NULL;
+		}
+
 		expr = opt_const(opt, false);
 		if (!expr) {
 			return NULL;
@@ -1987,7 +2018,9 @@ static struct bfs_expr *simplify_and(struct bfs_opt *opt, struct bfs_expr *expr,
 
 		if (ignore) {
 			opt_delete(opt, "%pe [ignored result]\n", child);
-			opt_warning(opt, child, "The result of this expression is ignored.\n\n");
+			if (!opt_ignore(opt, child)) {
+				return NULL;
+			}
 			continue;
 		}
 
@@ -2035,7 +2068,9 @@ static struct bfs_expr *simplify_or(struct bfs_opt *opt, struct bfs_expr *expr,
 
 		if (ignore) {
 			opt_delete(opt, "%pe [ignored result]\n", child);
-			opt_warning(opt, child, "The result of this expression is ignored.\n\n");
+			if (!opt_ignore(opt, child)) {
+				return NULL;
+			}
 			continue;
 		}
 
@@ -2076,7 +2111,9 @@ static struct bfs_expr *simplify_comma(struct bfs_opt *opt, struct bfs_expr *exp
 
 		if (opt->level >= 2 && child->pure && !SLIST_EMPTY(&children)) {
 			opt_delete(opt, "%pe [ignored result]\n", child);
-			opt_warning(opt, child, "The result of this expression is ignored.\n\n");
+			if (!opt_ignore(opt, child)) {
+				return NULL;
+			}
 			continue;
 		}
 
diff --git a/src/parse.c b/src/parse.c
index 7ba9e33..c890fa8 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -78,8 +78,6 @@ struct bfs_parser {
 
 	/** Whether stdout is a terminal. */
 	bool stdout_tty;
-	/** Whether this session is interactive (stdin and stderr are each a terminal). */
-	bool interactive;
 	/** Whether -color or -nocolor has been passed. */
 	enum use_color use_color;
 	/** Whether a -print action is implied. */
@@ -1185,8 +1183,12 @@ static struct bfs_expr *parse_daystart(struct bfs_parser *parser, int arg1, int
  * Parse -delete.
  */
 static struct bfs_expr *parse_delete(struct bfs_parser *parser, int arg1, int arg2) {
-	parser->ctx->flags |= BFTW_POST_ORDER;
+	struct bfs_ctx *ctx = parser->ctx;
+	ctx->flags |= BFTW_POST_ORDER;
+	ctx->dangerous = true;
+
 	parser->depth_arg = parser->argv;
+
 	return parse_nullary_action(parser, eval_delete);
 }
 
@@ -1246,7 +1248,9 @@ static struct bfs_expr *parse_empty(struct bfs_parser *parser, int arg1, int arg
  * Parse -exec(dir)?/-ok(dir)?.
  */
 static struct bfs_expr *parse_exec(struct bfs_parser *parser, int flags, int arg2) {
-	struct bfs_exec *execbuf = bfs_exec_parse(parser->ctx, parser->argv, flags);
+	struct bfs_ctx *ctx = parser->ctx;
+
+	struct bfs_exec *execbuf = bfs_exec_parse(ctx, parser->argv, flags);
 	if (!execbuf) {
 		return NULL;
 	}
@@ -1295,6 +1299,8 @@ static struct bfs_expr *parse_exec(struct bfs_parser *parser, int flags, int arg
 
 	if (execbuf->flags & BFS_EXEC_CONFIRM) {
 		parser->ok_expr = expr;
+	} else {
+		ctx->dangerous = true;
 	}
 
 	return expr;
@@ -3237,6 +3243,8 @@ static const struct table_entry *table_lookup_fuzzy(const char *arg) {
  *         | ACTION
  */
 static struct bfs_expr *parse_primary(struct bfs_parser *parser) {
+	struct bfs_ctx *ctx = parser->ctx;
+
 	// Paths are already skipped at this point
 	const char *arg = parser->argv[0];
 
@@ -3259,7 +3267,7 @@ static struct bfs_expr *parse_primary(struct bfs_parser *parser) {
 
 	match = table_lookup_fuzzy(arg);
 
-	CFILE *cerr = parser->ctx->cerr;
+	CFILE *cerr = ctx->cerr;
 	parse_error(parser, "Unknown argument; did you mean ");
 	switch (match->info & T_TYPE) {
 	case T_FLAG:
@@ -3273,7 +3281,7 @@ static struct bfs_expr *parse_primary(struct bfs_parser *parser) {
 		break;
 	}
 
-	if (!parser->interactive || !match->parse) {
+	if (!ctx->interactive || !match->parse) {
 		fprintf(stderr, "\n");
 		goto unmatched;
 	}
@@ -3483,6 +3491,8 @@ static struct bfs_expr *parse_expr(struct bfs_parser *parser) {
  * Parse the top-level expression.
  */
 static struct bfs_expr *parse_whole_expr(struct bfs_parser *parser) {
+	struct bfs_ctx *ctx = parser->ctx;
+
 	if (skip_paths(parser) != 0) {
 		return NULL;
 	}
@@ -3529,13 +3539,13 @@ static struct bfs_expr *parse_whole_expr(struct bfs_parser *parser) {
 			parser->xdev_arg[0], parser->mount_arg[0]);
 	}
 
-	if (parser->ctx->warn && parser->depth_arg && parser->prune_arg) {
+	if (ctx->warn && parser->depth_arg && parser->prune_arg) {
 		parse_conflict_warning(parser, parser->depth_arg, 1, parser->prune_arg, 1,
 			"${blu}%s${rs} does not work in the presence of ${blu}%s${rs}.\n",
 			parser->prune_arg[0], parser->depth_arg[0]);
 
-		if (parser->interactive) {
-			bfs_warning(parser->ctx, "Do you want to continue? ");
+		if (ctx->interactive) {
+			bfs_warning(ctx, "Do you want to continue? ");
 			if (ynprompt() == 0) {
 				return NULL;
 			}
@@ -3770,6 +3780,7 @@ struct bfs_ctx *bfs_parse_cmdline(int argc, char *argv[]) {
 	} else {
 		ctx->warn = stdin_tty;
 	}
+	ctx->interactive = stdin_tty && stderr_tty;
 
 	struct bfs_parser parser = {
 		.ctx = ctx,
@@ -3777,7 +3788,6 @@ struct bfs_ctx *bfs_parse_cmdline(int argc, char *argv[]) {
 		.command = ctx->argv[0],
 		.regex_type = BFS_REGEX_POSIX_BASIC,
 		.stdout_tty = stdout_tty,
-		.interactive = stdin_tty && stderr_tty,
 		.use_color = use_color,
 		.implicit_print = true,
 		.implicit_root = true,
@@ -3812,7 +3822,9 @@ struct bfs_ctx *bfs_parse_cmdline(int argc, char *argv[]) {
 	}
 
 	if (bfs_optimize(ctx) != 0) {
-		bfs_perror(ctx, "bfs_optimize()");
+		if (errno != 0) {
+			bfs_perror(ctx, "bfs_optimize()");
+		}
 		goto fail;
 	}