as jca@ says, the clearerr() should be out of the loop, so ok benno@ too.

J??r??mie Courr??ges-Anglas(j...@wxcvbn.org) on 2015.12.29 19:18:55 +0100:
> "Todd C. Miller" <todd.mil...@courtesan.com> writes:
> 
> > On Tue, 29 Dec 2015 13:25:16 +0100, 
> > =?utf-8?Q?J=C3=A9r=C3=A9mie_Courr=C3=A8ges-
> > Anglas?= wrote:
> >
> >> I think it makes sense to try to recover, so calling clearerr() is
> >> needed.  But as said by millert you can't rely on fprintf to set the
> >> error indicator; the write might not be committed to disk, and the
> >> ftruncate and fsync calls below won't magically update the FILE's error
> >> indicator.
> >> 
> >> What about using "r = fflush(freqfp);" instead of ferror?
> >
> > How does this look?
> >
> >  - todd
> >
> > Index: ntpd.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/ntpd/ntpd.c,v
> > retrieving revision 1.101
> > diff -u -p -u -r1.101 ntpd.c
> > --- ntpd.c  19 Dec 2015 17:55:29 -0000      1.101
> > +++ ntpd.c  29 Dec 2015 17:51:56 -0000
> > @@ -552,12 +552,12 @@ writefreq(double d)
> >     if (freqfp == NULL)
> >             return 0;
> >     rewind(freqfp);
> > -   fprintf(freqfp, "%.3f\n", d * 1e6);     /* scale to ppm */
> > -   r = ferror(freqfp);
> > -   if (r != 0) {
> > +   r = fprintf(freqfp, "%.3f\n", d * 1e6); /* scale to ppm */
> > +   if (r < 0 || fflush(freqfp) != 0) {
> 
> Fine with me.
> 
> >             if (warnonce) {
> >                     log_warnx("can't write %s", DRIFTFILE);
> >                     warnonce = 0;
> > +                   clearerr(freqfp);
> >             }
> 
> Looking closer, the clearerr() call should probably be out of the
> conditional.
> 
> With that fixed, ok jca@
> 
> >             return 0;
> >     }
> >
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 

-- 

Reply via email to