Hi Miroslav,
On Wed, Nov 29, 2023 at 9:05 PM Miroslav Lichvar <mlich...@redhat.com> wrote:
>
> On Wed, Nov 29, 2023 at 11:15:59AM +1100, patrick.oppenlan...@gmail.com 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.
In hindsight I should have marked this as an RFC patch.
> > /* ================================================== */
> >
> > +/* 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.
Thanks for the feedback, I'll prepare a v2.
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.
