On Fri, Mar 06 2020, "Theo de Raadt" <[email protected]> wrote:
>> Jeremie Courreges-Anglas <[email protected]> wrote:
>> >
>> >
>> > Checking the return value of fprintf is good but not enough to ensure
>> > that data has properly been written to the file without an error. To do
>> > that we can use fflush(3) in a single place.
[redacted]
>> How about checking the return value of fclose() in output_finish() instead?
> Oh you want to reach the error-reporting code...
And the cleanup-on-error code. Doing it here looks more appealing than
adding
if (fflush(out) != 0)
return -1;
at the end of all the output_* functions.
If you're careful about write errors you can avoid feeding an
incomplete/garbage file to your BGP daemon. The code was already
careful, but did not account for buffering. This is what this patch
tries to address.
>> > Build-tested only. ok?
>> >
>> > Bonus: in output_finish(), "out = NULL;" is pointless, so zap it.
>> > I suspect it's a remnant from a time where the output FILE * was
>> > a global. That would be a separate commit.
Diff below again for convenience,
Index: output.c
===================================================================
RCS file: /d/cvs/src/usr.sbin/rpki-client/output.c,v
retrieving revision 1.6
diff -u -p -p -u -r1.6 output.c
--- output.c 6 Mar 2020 17:36:42 -0000 1.6
+++ output.c 6 Mar 2020 23:04:18 -0000
@@ -71,7 +71,7 @@ outputfiles(struct vrp_tree *v)
rc = 1;
continue;
}
- if ((*outputs[i].fn)(fout, v) != 0) {
+ if ((*outputs[i].fn)(fout, v) != 0 || fflush(fout) != 0) {
logx("output for %s format failed", outputs[i].name);
fclose(fout);
output_cleantmp();
@@ -112,8 +112,6 @@ void
output_finish(FILE *out)
{
fclose(out);
- out = NULL;
-
rename(output_tmpname, output_name);
output_tmpname[0] = '\0';
}
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
