Page MenuHomeFreeBSD
Differential D30183 Diff 92432 contrib/tzcode/stdtime/localtime.c

Changeset View

Standalone View

View Options

contrib/tzcode/stdtime/localtime.c

Show First 20 Lines • Show All 348 Lines • ▼ Show 20 Lines for (i = 0; i < sp->typecnt; ++i) {
register char * cp = &sp->chars[ttisp->tt_abbrind]; register char * cp = &sp->chars[ttisp->tt_abbrind];
if (strlen(cp) > TZ_ABBR_MAX_LEN && if (strlen(cp) > TZ_ABBR_MAX_LEN &&
strcmp(cp, GRANDPARENTED) != 0) strcmp(cp, GRANDPARENTED) != 0)
*(cp + TZ_ABBR_MAX_LEN) = '\0'; *(cp + TZ_ABBR_MAX_LEN) = '\0';
} }
} }
#ifdef DETECT_TZ_CHANGES
/*
* Determine if there's a change in the timezone since the last time we checked.
* Returns: -1 on error
* 0 if the timezone has not changed
* 1 if the timezone has changed
*/
static int static int
change_in_tz(const char *name)
{
static char old_name[PATH_MAX];
static struct stat old_sb;
struct stat sb;
int error;
error = stat(name, &sb);
if (error != 0)
return -1;
if (strcmp(name, old_name) != 0) {
strlcpy(old_name, name, sizeof(old_name));
impUnsubmitted
Not Done
Inline Actions

Nit: This will always fail if old_name >= PATH_MAX and always trigger, but at least its fail safe: we'll just do extra work.

imp: Nit: This will always fail if old_name >= PATH_MAX and always trigger, but at least its fail…
old_sb = sb;
return 1;
}
if (sb.st_dev != old_sb.st_dev ||
sb.st_ino != old_sb.st_ino ||
sb.st_ctime != old_sb.st_ctime ||
sb.st_mtime != old_sb.st_mtime) {
old_sb = sb;
return 1;
}
return 0;
}
#else /* !DETECT_TZ_CHANGES */
#define change_in_tz(X) 0
#endif /* !DETECT_TZ_CHANGES */
static int
differ_by_repeat(const time_t t1, const time_t t0) differ_by_repeat(const time_t t1, const time_t t0)
{ {
int_fast64_t _t0 = t0; int_fast64_t _t0 = t0;
int_fast64_t _t1 = t1; int_fast64_t _t1 = t1;
if (TYPE_INTEGRAL(time_t) && if (TYPE_INTEGRAL(time_t) &&
TYPE_BIT(time_t) - TYPE_SIGNED(time_t) < SECSPERREPEAT_BITS) TYPE_BIT(time_t) - TYPE_SIGNED(time_t) < SECSPERREPEAT_BITS)
return 0; return 0;
//turn ((int_fast64_t)(t1 - t0) == SECSPERREPEAT); //turn ((int_fast64_t)(t1 - t0) == SECSPERREPEAT);
return _t1 - _t0 == SECSPERREPEAT; return _t1 - _t0 == SECSPERREPEAT;
} }
static int static int
tzload(name, sp, doextend) tzload(name, sp, doextend)
const char * name; const char * name;
struct state * const sp; struct state * const sp;
register const int doextend; register const int doextend;
{ {
const char * p; const char * p;
int i; int i;
int fid; int fid;
int stored; int stored;
int nread; int nread;
int res; int res;
int ret;
union { union {
struct tzhead tzhead; struct tzhead tzhead;
char buf[2 * sizeof(struct tzhead) + char buf[2 * sizeof(struct tzhead) +
2 * sizeof *sp + 2 * sizeof *sp +
4 * TZ_MAX_TIMES]; 4 * TZ_MAX_TIMES];
} *u; } *u;
u = NULL; u = NULL;
Show All 32 Lines if (name[0] != '/') {
free(fullname); free(fullname);
return -1; return -1;
} }
(void) strcpy(fullname, p); (void) strcpy(fullname, p);
(void) strcat(fullname, "/"); (void) strcat(fullname, "/");
(void) strcat(fullname, name); (void) strcat(fullname, name);
name = fullname; name = fullname;
} }
if (doextend == TRUE) {
/*
* Detect if the timezone file has changed. Check
* 'doextend' to ignore TZDEFRULES; the change_in_tz()
* function can only keep state for a single file.
*/
ret = change_in_tz(name);
impUnsubmitted
Not Done
Inline Actions

where is this function defined?

imp: where is this function defined?
if (ret <= 0) {
/*
* Returns -1 if there was an error,
* and 0 if the timezone had not changed.
*/
free(fullname);
return ret;
}
}
if ((fid = _open(name, OPEN_MODE)) == -1) { if ((fid = _open(name, OPEN_MODE)) == -1) {
free(fullname); free(fullname);
return -1; return -1;
} }
if ((_fstat(fid, &stab) < 0) || !S_ISREG(stab.st_mode)) { if ((_fstat(fid, &stab) < 0) || !S_ISREG(stab.st_mode)) {
free(fullname); free(fullname);
_close(fid); _close(fid);
return -1; return -1;
▲ Show 20 Lines • Show All 766 Lines • ▼ Show 20 Lines
static void static void
gmtload(struct state *const sp) gmtload(struct state *const sp)
{ {
if (tzload(gmt, sp, TRUE) != 0) if (tzload(gmt, sp, TRUE) != 0)
(void) tzparse(gmt, sp, TRUE); (void) tzparse(gmt, sp, TRUE);
} }
#ifdef DETECT_TZ_CHANGES
static int
recheck_tzdata()
{
static time_t last_checked;
struct timespec now;
time_t current_time;
int error;
/*
* We want to recheck the timezone file every 61 sec.
* The 'last_checked' may be greater than 'current_time'
* if time is set backwards.
impUnsubmitted
Not Done
Inline Actions

This comment is now false and should be removed.

imp: This comment is now false and should be removed.
*/
impUnsubmitted
Not Done
Inline Actions

This is about 1000x too often to check the timezone changes. The files change on a weekly basis. Checking them once a day should suffice and even it's likely 10x overkill, or once an hour if you really are into overkill. Please make it a #define at least if you insist that once a minute is needed for your company so the project can have a less vigorous check since every single daemon that's getting time will be hammering here.

Also, time is the wrong function to use here. Use clock_gettime with CLOCK_MONOTONIC. Then you don't have to worry about time going backwards or have this weird convoluted code. It's 100% guaranteed to be monotonic, and the cheapest way to get the time from the system. There's all manor of other weird pathology this avoids as well, which this code doesn't do. Given the level of precision, you can ignore the fractions of a second...

imp: This is about 1000x too often to check the timezone changes. The files change on a weekly basis.
impUnsubmitted
Not Done
Inline Actions

I mean 5k too often. There's about 10k minutes in a week. These files change on the average once every few weeks upstream (well, on busy years, other years it is closer to monthly or less). While there are some high-priority changes once in a blue moon when some country announces at the last minute they won't do DST this year (or come off DST), those are adequately handled by polling once a day (every 1440 minutes) which has a lot of nice prime numbers around 86,400, though to avoid a thundering heard problem on systems with lots of daemons, some bit of randomness should likely be introduced as well.. Coupled with the idea of fetching these once a day automatically, this would produce a minimal number of extra stat calls.... If you're trying to cover cases where the user upgrades, then most of the time, the new zone files aren't needed today (with rare exceptions).... If you are trying to cover the case where the user changes the system's timezone, then a different notification mechanism (ala kqueue on the zone file with a notification for when it changes) would be in order.

imp: I mean 5k too often. There's about 10k minutes in a week. These files change on the average…
error = clock_gettime(CLOCK_MONOTONIC_FAST, &now);
impUnsubmitted
Not Done
Inline Actions

I'd use a plain MONOTONIC here. There's no real reason to use the _FAST version here. On modern hardware, this would save, at most, a rdtsc invocation. Plus the CLOCK_MONOTONIC is POSIX standard, so is more likely to be accepted upstream.

imp: I'd use a plain MONOTONIC here. There's no real reason to use the _FAST version here. On modern…
impUnsubmitted
Not Done
Inline Actions

as an aside, I don't think this can fail on FreeBSD.

imp: as an aside, I don't think this can fail on FreeBSD.
if (error <= 0) {
/* XXX: Can we somehow report this? */
return 0;
}
current_time = now.tv_sec;
if ((current_time - last_checked > 61) ||
impUnsubmitted
Not Done
Inline Actions

I wonder if there's an advantage to having this be aligned among all programs in the system to get better caching from all the processes reading this data.

imp: I wonder if there's an advantage to having this be aligned among all programs in the system to…
traszAuthorUnsubmitted
Done
Inline Actions

That sounds like something that would be nice, but I think it would require quite significant changes :-)

trasz: That sounds like something that would be nice, but I think it would require quite significant…
(last_checked > current_time)) {
last_checked = current_time;
return 1;
}
return 0;
}
#else /* !DETECT_TZ_CHANGES */
#define recheck_tzdata() 0
#endif /* !DETECT_TZ_CHANGES */
static void static void
tzsetwall_basic(int rdlocked) tzsetwall_basic(int rdlocked)
{ {
if (!rdlocked) if (!rdlocked)
_RWLOCK_RDLOCK(&lcl_rwlock); _RWLOCK_RDLOCK(&lcl_rwlock);
if (lcl_is_set < 0) { if (lcl_is_set < 0 && recheck_tzdata() == 0) {
impUnsubmitted
Not Done
Inline Actions

This looks like it will re-read things once a minute with tzload. Am I reading this wrong? I don't see where that's doing any kind of stat to make sure that the file has actually changed. Shouldn't we at the very least lstat TZDEFAULT here to see if its contents have changed somewhere?

imp: This looks like it will re-read things once a minute with tzload. Am I reading this wrong? I…
impUnsubmitted
Not Done
Inline Actions

Doh! I meant to edit this: I see where the check was added. It isn't in the current version which I looked at.

imp: Doh! I meant to edit this: I see where the check was added. It isn't in the current version…
traszAuthorUnsubmitted
Done
Inline Actions

The stat(2) check is inside tzload(); look for change_in_tz().

trasz: The stat(2) check is inside tzload(); look for change_in_tz().
if (!rdlocked) if (!rdlocked)
_RWLOCK_UNLOCK(&lcl_rwlock); _RWLOCK_UNLOCK(&lcl_rwlock);
return; return;
} }
_RWLOCK_UNLOCK(&lcl_rwlock); _RWLOCK_UNLOCK(&lcl_rwlock);
_RWLOCK_WRLOCK(&lcl_rwlock); _RWLOCK_WRLOCK(&lcl_rwlock);
lcl_is_set = -1; lcl_is_set = -1;
▲ Show 20 Lines • Show All 1,005 Lines • Show Last 20 Lines