On 2022-09-18 01:55 -04, Paul Janzen <pjan...@foatdi.net> wrote:
> The recent change to grdc(6), to display additional information if TZ is
> set, has a few issues.
>
> 1. Time zone offset incorrectly reported in Newfoundland.
>
> Some time zones have offsets of 30 or 45 minutes. The displayed time
> offset is currently truncated to the closest hour. (Australia/Eucla
> is a fun time zone too.)
Very good point. Let's shine this turd a bit more!
>
> 2. The new, additional information disappears if the window is sized too
> small (wintoosmall). There's basically room for it though... except
I don't care too much about the wintoosmall case, so I left your version
in. I guess we could do better with the TZ validation (see below).
>
> 3. The TZ information is a string of unknown length, and so it doesn't
> necessarily display correctly.
>
> Yeah, I know with pledge() you can't do testing like
> TZ=:/home/pjanzen/dev/usr/share/zoneinfo/testing/2022/America/Kentucky/Monticello
> any more, even though that's a perfectly cromulent and functional TZ on my
> system otherwise. So for the time being, probably any TZ that exists and
> doesn't abort grdc is 38 bytes or shorter. Which works.
grdc got aborted even before this last change. That's not optimal. So we
have to hoist the (implicit) call to tzset before pledge(2). I went
further and did:
pledge("stdio rpath tty")
tzset()
pledge("stdio tty")
>
> But, if your timezone exists, it already has its name in short form
> included in tm->tm_zone. That's what I think should be
> printed, even if "EDT" is way less cool than "America/Pangnirtung". I
> mean, if the point is just to be able to label clocks with nice places,
> instead of the time zone it's showing, maybe it could be a different
> option.
For me, that would completely miss the point. I have no idea what EDT
means.
>
> Counterpoint: some of the timezones have short names that are just the UTC
> offset, which is really boring and duplicated given that we print out the
> offset already. Hey, maybe we could just print the offset...
>
> 4. There's no indication if you type an invalid TZ--you get UTC and a
> misleading label onscreen.
>
> Timezone handling defaults to UTC if anything breaks along the chain, as
> the tzset(3) man page makes clear. That means if you misspell Antarctica
> while setting your TZ=Antartica/McMurdo, you'll end up half a day off from
> all your pals at McMurdo as your screen happily tells you that you're
> seeing McMurdo +0 time.
>
> There's no simple way to tell if your $TZ is valid or not (if you get back
> UTC, maybe that's really what it should be!). At least if we display
> tm->tm_zone rather than $TZ, we're not misleading.
>
Right, so I had a stab at validating TZ:
If TZ is a relative path and exists as a file in /usr/share/zoneinfo
display it and also print tm->tm_zone:
┌[ Antarctica/McMurdo (NZST) +12h00 ]──────────────────────┐
┌[ Australia/Eucla (+0845) +8h45 ]─────────────────────────┐
There is a TOCTU issue (doesn't matter I think) and an issue where what
we find in /usr/share/zoneinfo is not syntactically correct, i.e.:
$ TZ=zone.tab grdc
┌[ zone.tab (GMT) +0h00 ]──────────────────────────────────┐
I don't think that matters either. Both are cases of: don't do that.
>
>
> I was going to be upset that the man page for grdc(6) is way pickier on TZ
> than tzset(3), but it's quite accurate given the pledge() call. Speaking
> of which, I don't expect any one else plays with timezone files, and surely
> one doesn't want one's general utilities to be pwned by possible bugs in
> the time-handling code exploited with custom files created outside
> /usr/share/zoneinfo. But it's still a touch irritating-should-be-fixable
> that, because the pledge() has to be after initscr(), grdc has the
> possibility of leaving the terminal in the wrong state when it aborts on
> test TZ files.
>
This works now, too:
$ TZ=~/Newfoundland grdc
┌[ NDT -2h30 ]─────────────────────────────────────────────┐
>
>
> Paul Janzen.
>
> Index: grdc.c
> ===================================================================
> RCS file: /cvs/src/games/grdc/grdc.c,v
> retrieving revision 1.35
> +void
> +print_tz(int y, int x, int sml)
> +{
> + int i, j;
> +
> + move(y, x);
> + i = tm->tm_gmtoff / 60 / 60;
> + j = tm->tm_gmtoff / 60 - i * 60;
Isn't this just a weird spelling for mod (%)?
> + if (i < 0)
> + j = -j;
> + if (!sml)
> + printw("[ %s %+dh%02d ]", tm->tm_zone, i, j);
> + else
> + printw("[%+dh%02d]", i, j);
> }
>
> void
>
diff --git grdc.c grdc.c
index 66e5eee79e6..5d2ea19a532 100644
--- grdc.c
+++ grdc.c
@@ -12,6 +12,7 @@
*/
#include <sys/ioctl.h>
+#include <sys/stat.h>
#include <curses.h>
#include <err.h>
@@ -26,9 +27,6 @@
#define XLENGTH 58
#define YDEPTH 7
-struct timespec now;
-struct tm *tm;
-
short disp[11] = {
075557, 011111, 071747, 071717, 055711,
074717, 074757, 071111, 075757, 075717, 002020
@@ -41,6 +39,7 @@ volatile sig_atomic_t sigwinched = 0;
int hascolor = 0;
+void print_tz(int, int, int, const struct tm *, const char *);
void getwinsize(int *, int *);
void set(int, int);
void standt(int);
@@ -67,17 +66,19 @@ sigresize(int signo)
int
main(int argc, char *argv[])
{
+ struct tm *tm;
long t, a;
int i, j, s, k, rv;
int scrol;
unsigned int n = 0;
- struct timespec delay;
+ struct timespec now, delay;
struct pollfd pfd;
const char *errstr;
long scroldelay = 50000000;
int xbase;
int ybase;
int wintoosmall;
+ int tz_valid = 0;
char *tz;
tz = getenv("TZ");
@@ -108,6 +109,34 @@ main(int argc, char *argv[])
initscr();
+ if (pledge("stdio rpath tty", NULL) == -1)
+ err(1, "pledge");
+
+ tzset();
+
+ if (tz != NULL && *tz != ':' && *tz != '/') {
+ int len;
+ char path[PATH_MAX];
+ /*
+ * Check if the time zone exists in time zone information
+ * directory. There is a time window between the tzset(3) call
+ * and checking if the file exists where the system
+ * administrator could have put the file there. We don't care,
+ * this is a game.
+ */
+ len = snprintf(path, sizeof(path), "/usr/share/zoneinfo/%s",
tz);
+ if (len < 0 || (size_t)len >= sizeof(path))
+ tz_valid = 0;
+ else {
+ struct stat sb;
+
+ if (stat(path, &sb) == -1)
+ tz_valid = 0;
+ else
+ tz_valid = S_ISREG(sb.st_mode);
+ }
+ }
+
if (pledge("stdio tty", NULL) == -1)
err(1, "pledge");
@@ -184,11 +213,9 @@ main(int argc, char *argv[])
move(ybase, xbase + XLENGTH);
vline(ACS_VLINE, YDEPTH);
- if (tz != NULL) {
- move(ybase - 1, xbase);
- printw("[ %s %+d ]", tz,
- tm->tm_gmtoff / 60 / 60 );
- }
+ if (tz != NULL)
+ print_tz(ybase - 1, xbase, wintoosmall,
+ tm, tz_valid ? tz : NULL);
attrset(COLOR_PAIR(2));
}
@@ -199,6 +226,9 @@ main(int argc, char *argv[])
move(0, 0);
printw("%02d:%02d:%02d", tm->tm_hour, tm->tm_min,
tm->tm_sec);
+ if (tz != NULL)
+ print_tz(1, 0, wintoosmall, tm,
+ tz_valid ? tz : NULL);
} else for (k = 0; k < 6; k++) {
if (scrol) {
for(i = 0; i < 5; i++)
@@ -284,6 +314,23 @@ set(int t, int n)
mask |= m;
}
+void
+print_tz(int y, int x, int sml, const struct tm *tm, const char *tz)
+{
+ int h, m;
+
+ move(y, x);
+ h = tm->tm_gmtoff / 3600;
+ m = abs((int)tm->tm_gmtoff % 3600 / 60);
+ if (!sml) {
+ if (tz != NULL)
+ printw("[ %s (%s) %+dh%02d ]", tz, tm->tm_zone, h, m);
+ else
+ printw("[ %s %+dh%02d ]", tm->tm_zone, h, m);
+ } else
+ printw("[%+dh%02d]", h, m);
+}
+
void
standt(int on)
{
--
I'm not entirely sure you are real.
