Re: [chrony-dev] [PATCH 1/1] reference: support leap-seconds.list leap second source

[Miroslav Lichvar <mlichvar@xxxxxxxxxx>]

On Wed, Nov 29, 2023 at 11:15:59AM +1100, patrick.oppenlander@xxxxxxxxx wrote:
> This patch adds support for getting leap second information from the
> leap-seconds.list file included with tzdata and adds a new configuration
> directive leapsecdb to switch on the feature.

Good idea.

I have some comments on the design first. Details can be sorted out
later. Please don't expect this to be ready on first attempt.

>  /* ================================================== */
>  
> +/* Leap second database */
> +struct leapdb {
> +  long long updated;
> +  long long expiry;
> +  size_t len;
> +  struct leap {
> +    long long when;
> +    int tai_offset;
> +  } leap[];
> +};

No need to keep this information in memory. Just read the file for
each request. It's only twice per day.

> +  if (leap_db) {
> +    /* Check that the leap database has good data for Jun 30 2012 and Dec 31 2012 */
> +    if (get_db_leap(1341014400, &tai_offset) == LEAP_InsertSecond && tai_offset == 34 &&
> +        get_db_leap(1356912000, &tai_offset) == LEAP_Normal && tai_offset == 35) {
> +      LOG(LOGS_INFO, "Using leap second database %s", leap_db);
> +    } else {
> +      LOG(LOGS_WARN, "Leap second database %s failed check, ignoring", leap_db);
> +      leap_db = NULL;
> +    }
> +  }

Code like this should not be duplicated. I think there should be a
wrapper around get_tz_leap() and get_db_leap() implementing the
12-hour caching logic and the callers shouldn't care which database is
used.

Also, this is not really relevant to "reference", so it might a good
opportunity to move it out to separate file, e.g. leapdb.c. The API
would be just three functions LDB_Initialise(), LDB_GetLeap(),
LDB_Finalise(). The first commit would refactor the existing code into
that. The second commit would add the leapfile parsing, documentation
and some tests.

-- 
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.