From 1234d764ba3059eb2314cb50a2790240ed7ac630 Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Tue, 2 Jul 2024 16:16:32 -0400 Subject: 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 --- src/ctx.h | 9 +++++++-- src/opt.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++------------ src/parse.c | 34 +++++++++++++++++++++++----------- 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; } -- cgit v1.2.3