Re: [PATCH v4 03/13] bisect--helper: introduce new `write_in_file()` function

From: Chris Torek <chris.torek@gmail.com>
Date: Jul 1, 2020, 14:56 UTC
Message-ID: <CAPx1GvfFCKqTN2zodrO767CQQ2t2m-s9SRwR+s3cs0oAWyJ8Fg@mail.gmail.com>
In-Reply-To: <20200701133504.18360-4-mirucam@gmail.com>

On Wed, Jul 1, 2020 at 6:37 AM Miriam Rubio <mirucam@gmail.com> wrote:

> Let's refactor code adding a new `write_in_file()` function
> that opens a file for writing a message and closes it and a
> wrapper for writting mode.

Nit: typo, s/writting/writing/

[snippage]

Show 15 quoted lines

> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 0466b07a43..b421056546 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -74,6 +74,38 @@ static int one_of(const char *term, ...)
>         return res;
>  }
>
> +static int write_in_file(const char *path, const char *mode, const char *format, va_list args)
> +{
> +       FILE *fp = NULL;
> +       int res = 0;
> +
> +       if (!strcmp(mode, "a") && !strcmp(mode, "w"))
> +               return error(_("wrong writing mode '%s'"), mode);

Should this maybe just be BUG()?

Show 6 quoted lines

> +       fp = fopen(path, mode);
> +       if (!fp)
> +               return error_errno(_("cannot open file '%s' in mode '%s'"), path, mode);
> +       res = vfprintf(fp, format, args);
> +
> +       if (!res) {

This isn't quite right - vfprintf() returns a negative value on error, or the number of characters printed on success. Zero is technically OK (if the format and arguments ended up empty).

[rest snipped]

Chris