Re: [chrony-dev] [PATCH v4 4/5] leapdb: support leap-seconds.list as second source

[Miroslav Lichvar <mlichvar@xxxxxxxxxx>]

On Wed, Feb 07, 2024 at 04:29:43PM +1100, patrick.oppenlander@xxxxxxxxx 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-request@xxxxxxxxxxxxxxxxxxxx with "unsubscribe" in the subject.
For help email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "help" in the subject.
Trouble?  Email listmaster@xxxxxxxxxxxxxxxxxxxx.