Jeremie Courreges-Anglas <[email protected]> wrote:
Lovely!
> Agreed. Here's an updated diff that tests the return value of
> output_finish(). Suggestions welcome for the error message in this
> case.
>
> It also drops the extra "out = NULL;", and replaces logx() calls with
> warn(3) in this file. I see no reason to drop those messages and errno
> information. logx() seems used mostly for statistics.
>
> Thoughts, tests / oks?
>
>
> Index: extern.h
> ===================================================================
> RCS file: /d/cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.24
> diff -u -p -r1.24 extern.h
> --- extern.h 6 Mar 2020 17:36:42 -0000 1.24
> +++ extern.h 9 Mar 2020 08:19:20 -0000
> @@ -372,7 +372,7 @@ extern char* outputdir;
> int outputfiles(struct vrp_tree *v);
> FILE *output_createtmp(char *);
> void output_cleantmp(void);
> -void output_finish(FILE *);
> +int output_finish(FILE *);
> int output_bgpd(FILE *, struct vrp_tree *);
> int output_bird1v4(FILE *, struct vrp_tree *);
> int output_bird1v6(FILE *, struct vrp_tree *);
> Index: output.c
> ===================================================================
> RCS file: /d/cvs/src/usr.sbin/rpki-client/output.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 output.c
> --- output.c 6 Mar 2020 17:36:42 -0000 1.6
> +++ output.c 9 Mar 2020 08:20:39 -0000
> @@ -67,18 +67,23 @@ outputfiles(struct vrp_tree *v)
>
> fout = output_createtmp(outputs[i].name);
> if (fout == NULL) {
> - logx("cannot create %s", outputs[i].name);
> + warn("cannot create %s", outputs[i].name);
> rc = 1;
> continue;
> }
> if ((*outputs[i].fn)(fout, v) != 0) {
> - logx("output for %s format failed", outputs[i].name);
> + warn("output for %s format failed", outputs[i].name);
> fclose(fout);
> output_cleantmp();
> rc = 1;
> continue;
> }
> - output_finish(fout);
> + if (output_finish(fout) != 0) {
> + warn("finish for %s format failed", outputs[i].name);
> + output_cleantmp();
> + rc = 1;
> + continue;
> + }
> }
>
> return rc;
> @@ -108,14 +113,15 @@ output_createtmp(char *name)
> return f;
> }
>
> -void
> +int
> output_finish(FILE *out)
> {
> - fclose(out);
> - out = NULL;
> -
> - rename(output_tmpname, output_name);
> + if (fclose(out) != 0)
> + return -1;
> + if (rename(output_tmpname, output_name) == -1)
> + return -1;
> output_tmpname[0] = '\0';
> + return 0;
> }
>
> void
