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. Everything else looks good to me. Thanks for your work on this. -- 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.