On Mon, Sep 13 2021, Miriam Rubio wrote:
> +static int print_file_to_stdout(const char *path)
> +{
> + int fd = open(path, O_RDONLY);
> + int ret = 0;
> +
> + if (fd < 0)
> + return error_errno(_("cannot open file '%s' for reading"), path);
> + if (copy_fd(fd, 1) < 0)
> + ret = error_errno(_("failed to read '%s'"), path);
> + close(fd);
> + return ret;
> +}
Returns int, but that return value is ignored here, and we don't seem to
gain a caller in 6/6?
> + if (argc)
> + sq_quote_argv(&command, argv);
> + else {
> + error(_("bisect run failed: no command provided."));
> + return BISECT_FAILED;
> + }
Not new in this series & I see this is v7 already, so ....
Just odd to see this BISECT_FAILED pattern (which is defined to -1),
instead of "return error(..." like elsewhere.
Then we take that enum and do a "return -res" from main(), i.e. it's not
even that we're somehow guarding everything with these BISECT_* codes in
this file (see 30276765c11 (bisect--helper: use '-res' in
'cmd_bisect__helper' return, 2020-08-28)).
Anyway, can be cleaned up some other time, but...
> + if (temporary_stdout_fd < 0)
> + return error_errno(_("cannot open file '%s' for writing"), git_path_bisect_run());
Here we're doing a "return error...(" directly.
> + case BISECT_RUN:
> + if (!argc)
> + return error(_("bisect run failed: no command provided."));
> + get_terms(&terms);
> + res = bisect_run(&terms, argv, argc);
> + break;
> default:
> BUG("unknown subcommand %d", cmdmode);
> }
Also not a new issue, but if we just covered the BISECT_AUTOSTART case
here, then we wouldn't need this default/BUG, the compiler would check
that we checked all existing enum arms.
