On Wed, Feb 7, 2024 at 9:35 PM Miroslav Lichvar <mlich...@redhat.com> wrote:
>
> On Wed, Feb 07, 2024 at 04:29:43PM +1100, patrick.oppenlan...@gmail.com wrote:
> > + while (fgets(line, sizeof line, f) > 0) {
> > + int64_t lsl_when;
> > + int lsl_tai_offset;
> > + char *p;
> > +
> > + /* Ignore blank lines */
> > + for (p = line; *p && isspace(*p); ++p);
> > + if (!*p)
> > + continue;
> > +
> > + if (*line == '#') {
> > + /* Update time line starts with #$ */
> > + if (line[1] == '$' && sscanf(line + 2, "%"SCNd64, &lsl_updated) != 1)
> > + goto error;
> > + /* Expiration time line starts with #@ */
> > + if (line[1] == '@' && sscanf(line + 2, "%"SCNd64, &lsl_expiry) != 1)
> > + goto error;
> > + /* Comment or a special comment we don't care about */
> > + continue;
> > + }
> > +
> > + /* Leap entry */
> > + if (sscanf(line, "%"SCNd64" %d", &lsl_when, &lsl_tai_offset) != 2)
> > + goto error;
> > +
> > + if (when == lsl_when) {
> > + if (lsl_tai_offset > prev_lsl_tai_offset)
> > + lsl_leap = LEAP_InsertSecond;
> > + else if (lsl_tai_offset < prev_lsl_tai_offset)
> > + lsl_leap = LEAP_DeleteSecond;
> > + /* When is rounded to the end of the day, so offset hasn't changed
> > yet! */
> > + *tai_offset = prev_lsl_tai_offset;
> > + } else if (when > lsl_when)
> > + *tai_offset = lsl_tai_offset;
> > +
> > + prev_lsl_tai_offset = lsl_tai_offset;
> > + }
> > +
> > + /* Make sure the file looks sensible */
> > + if (!feof(f) || !lsl_updated || !lsl_expiry)
> > + goto error;
>
> The error path taken here and in the loop logs an error message, but
> the leap and TAI offset values can already be accepted at that point.
> I think either they should be saved to temporary variables, or the
> loop should stop when a matching entry is found, so the error message
> is not logged when the result is used.
Thanks for pointing that out. I'll go with temporary variables. I
think it's better not to use the values if the file looks bogus.
> Everything else looks good to me. Thanks for your work on this.
Thanks for spending the time to review it,
Patrick
> --
> Miroslav Lichvar
>
>
> --
> To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with
> "unsubscribe" in the subject.
> For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the
> subject.
> Trouble? Email listmas...@chrony.tuxfamily.org.
>
--
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe"
in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the
subject.
Trouble? Email listmas...@chrony.tuxfamily.org.
