[tz] tzcode error handling (patch 3 of 4)

Steve Summit scs at eskimo.com
Mon Dec 11 12:37:20 UTC 2017


This is patch 3, which repurposes the local_storage union so that
it can also save the reason that an attempted zone load failed.
Doing so has several implications:

* local_storage changes from a union to a struct.  (This means
  tzcode will use a bit more memory, although not permanently so.)

* The allocation of local_storage is no longer performed in
  tzload, but rather in its callers: tzparse, gmtload, and
  zoneinit.  This means that tzload doesn't do anything any more,
  so I have merged it with tzloadbody.

* The allocation of local_storage in tzload used to be either
  on the heap or the stack, depending on whether ALL_STATE was
  defined.  For simplicity, I've removed the heap allocation
  option, such that now (in all three of tzparse, gmtload, and
  zoneinit), local_storage is always allocated on the stack.
  I don't understand why it was ever important to allocate it
  on the heap (even in the ALL_STATE case), since it was always
  freed immediately afterwards.  But I don't fully understand
  what ALL_STATE is for, so I might have missed something.

This patch is with respect to 2017c, plus Patches 1 and 2.

					Steve Summit
					scs at eskimo.com
-------------- next part --------------
--- a/localtime.c	2017-12-06 19:27:49.000000000 -0500
+++ b/localtime.c	2017-12-07 22:48:24.000000000 -0500
@@ -167,9 +167,7 @@
 #ifdef ALL_STATE
 static struct state *	lclptr;
 static struct state *	gmtptr;
-#endif /* defined ALL_STATE */
-
-#ifndef ALL_STATE
+#else
 static struct state	lclmem;
 static struct state	gmtmem;
 #define lclptr		(&lclmem)
@@ -211,10 +209,6 @@
 
 static void (*wrnfunc)(char const *, ...) = NULL;
 
-/* unfortunate statics; these will hinder reentrancy */
-static char const *tzlb_reason = NULL;
-static char tzlb_filename[FILENAME_MAX + 1];
-
 /* Initialize *S to a value based on GMTOFF, ISDST, and ABBRIND.  */
 static void
 init_ttinfo(struct ttinfo *s, int_fast32_t gmtoff, bool isdst, int abbrind)
@@ -370,8 +364,8 @@
 /* TZDIR with a trailing '/' rather than a trailing '\0'.  */
 static char const tzdirslash[sizeof TZDIR] = TZDIR "/";
 
-/* Local storage needed for 'tzloadbody'.  */
-union local_storage {
+/* Local storage needed for 'tzload'. */
+struct local_storage {
   /* The results of analyzing the file's contents after it is opened.  */
   struct file_analysis {
     /* The input buffer.  */
@@ -384,18 +378,22 @@
   /* The file name to be opened.  */
   char fullname[BIGGEST(sizeof (struct file_analysis),
 			sizeof tzdirslash + 1024)];
+
+  /* A reason that opening or parsing the file or tzname failed. */
+  const char *failreason;
 };
 
+static void tzfilerr(int, struct local_storage *);
+
 /* Load tz data from the file named NAME into *SP.  Read extended
    format if DOEXTEND.  Use *LSP for temporary storage.  Return 0 on
    success, an errno value on failure.  Also on failure,
-   tzlb_filename is the full pathname of the tzdatafile, and
-   tzlb_reason is a reason for the failure not necessarily
-   reflected by errno.  (Beware: these error-reporting aspects
-   are not thread-safe.) */
+   lsp->fullname is the full pathname of the failing tzdatafile,
+   and lsp->reason is a reason for the failure not necessarily
+   reflected by errno. */
 static int
-tzloadbody(char const *name, struct state *sp, bool doextend,
-	   union local_storage *lsp)
+tzload(char const *name, struct state *sp, bool doextend,
+	   struct local_storage *lsp)
 {
 	register int			i;
 	register int			fid;
@@ -407,13 +405,13 @@
 
 	sp->goback = sp->goahead = false;
 
-	tzlb_reason = NULL;
-	*tzlb_filename = '\0';
+	lsp->failreason = NULL;
+	*lsp->fullname = '\0';
 
 	if (! name) {
 		name = TZDEFAULT;
 		if (! name) {
-		  tzlb_reason = "missing zone name";
+		  lsp->failreason = "missing zone name";
 		  return EINVAL;
 		  }
 	}
@@ -421,7 +419,11 @@
 	if (name[0] == ':')
 		++name;
 	doaccess = name[0] == '/';
-	if (!doaccess) {
+	if (doaccess) {
+		if (sizeof lsp->fullname - 1 < strlen(name))
+		  return ENAMETOOLONG;
+		strcpy(lsp->fullname, name);	/* stash for diagnostics */
+	} else {
 		size_t namelen = strlen(name);
 		if (sizeof lsp->fullname - sizeof tzdirslash <= namelen)
 		  return ENAMETOOLONG;
@@ -437,8 +439,6 @@
 			doaccess = true;
 		name = lsp->fullname;
 	}
-	*tzlb_filename = '\0';		/* use strncat as safe strcpy */
-	strncat(tzlb_filename, name, sizeof(tzlb_filename)-1);
 	if (doaccess && access(name, R_OK) != 0)
 	  return errno;
 	fid = open(name, OPEN_MODE);
@@ -452,7 +452,7 @@
 	    err = errno;
 	  else {
 	    err = EINVAL;
-	    tzlb_reason = "truncated header";
+	    lsp->failreason = "truncated header";
 	  }
 	  close(fid);
 	  return err;
@@ -475,7 +475,7 @@
 		       && 0 <= charcnt && charcnt < TZ_MAX_CHARS
 		       && (ttisstdcnt == typecnt || ttisstdcnt == 0)
 		       && (ttisgmtcnt == typecnt || ttisgmtcnt == 0))) {
-		  tzlb_reason = "invalid counts";
+		  lsp->failreason = "invalid counts";
 		  return EINVAL;
 		}
 		if (nread
@@ -487,7 +487,7 @@
 		       + leapcnt * (stored + 4)	/* lsinfos */
 		       + ttisstdcnt		/* ttisstds */
 		       + ttisgmtcnt)) {		/* ttisgmts */
-		  tzlb_reason = "insufficient data";
+		  lsp->failreason = "insufficient data";
 		  return EINVAL;
 		}
 		sp->leapcnt = leapcnt;
@@ -509,7 +509,7 @@
 			       ? TIME_T_MIN : at);
 			  if (timecnt && attime <= sp->ats[timecnt - 1]) {
 			    if (attime < sp->ats[timecnt - 1]) {
-			      tzlb_reason = "timecnt err";
+			      lsp->failreason = "timecnt err";
 			      return EINVAL;
 			    }
 			    sp->types[i - 1] = 0;
@@ -524,7 +524,7 @@
 		for (i = 0; i < sp->timecnt; ++i) {
 			unsigned char typ = *p++;
 			if (sp->typecnt <= typ) {
-			  tzlb_reason = "timecnt err 2";
+			  lsp->failreason = "timecnt err 2";
 			  return EINVAL;
 			}
 			if (sp->types[i])
@@ -540,13 +540,13 @@
 			p += 4;
 			isdst = *p++;
 			if (! (isdst < 2)) {
-			  tzlb_reason = "invalid isdst val";
+			  lsp->failreason = "invalid isdst val";
 			  return EINVAL;
 			}
 			ttisp->tt_isdst = isdst;
 			abbrind = *p++;
 			if (! (abbrind < sp->charcnt)) {
-			  tzlb_reason = "invalid abbrind";
+			  lsp->failreason = "invalid abbrind";
 			  return EINVAL;
 			}
 			ttisp->tt_abbrind = abbrind;
@@ -572,7 +572,7 @@
 		       second.  */
 		    if (tr - prevtr < 28 * SECSPERDAY - 1
 			|| (corr != prevcorr - 1 && corr != prevcorr + 1)) {
-		      tzlb_reason = "invalid leapsec data";
+		      lsp->failreason = "invalid leapsec data";
 		      return EINVAL;
 		    }
 		    sp->lsis[leapcnt].ls_trans = prevtr = tr;
@@ -590,7 +590,7 @@
 				ttisp->tt_ttisstd = false;
 			else {
 				if (*p != true && *p != false) {
-				  tzlb_reason = "invalid ttisstd val";
+				  lsp->failreason = "invalid ttisstd val";
 				  return EINVAL;
 				}
 				ttisp->tt_ttisstd = *p++;
@@ -604,7 +604,7 @@
 				ttisp->tt_ttisgmt = false;
 			else {
 				if (*p != true && *p != false) {
-				  tzlb_reason = "invalid ttisgmt val";
+				  lsp->failreason = "invalid ttisgmt val";
 				  return EINVAL;
 				}
 				ttisp->tt_ttisgmt = *p++;
@@ -735,26 +735,6 @@
 	return 0;
 }
 
-/* Load tz data from the file named NAME into *SP.  Read extended
-   format if DOEXTEND.  Return 0 on success, an errno value on failure.  */
-static int
-tzload(char const *name, struct state *sp, bool doextend)
-{
-#ifdef ALL_STATE
-  union local_storage *lsp = malloc(sizeof *lsp);
-  if (!lsp)
-    return errno;
-  else {
-    int err = tzloadbody(name, sp, doextend, lsp);
-    free(lsp);
-    return err;
-  }
-#else
-  union local_storage ls;
-  return tzloadbody(name, sp, doextend, &ls);
-#endif
-}
-
 static bool
 typesequiv(const struct state *sp, int a, int b)
 {
@@ -1080,6 +1060,7 @@
 	register char *			cp;
 	int				err;
 	register bool			load_ok;
+	struct local_storage		ls;  /* need to malloc if ALL_STATE? */
 
 	stdname = name;
 	if (lastditch) {
@@ -1108,18 +1089,12 @@
 	charcnt = stdlen + 1;
 	if (sizeof sp->chars < charcnt)
 	  return TZP_PARSE_ERR;
-	err = tzload(TZDEFRULES, sp, false);
+	err = tzload(TZDEFRULES, sp, false, &ls);
 	load_ok = (err == 0);
 	if (!load_ok) {
 		if(wrnfunc) {
 		    (*wrnfunc)("can't load default rules %s", TZDEFRULES);
-		    if(*tzlb_filename) {
-			if(tzlb_reason)
-			    (*wrnfunc)("%s: %s", tzlb_filename, tzlb_reason);
-			if(!tzlb_reason || err != EINVAL)
-			    (*wrnfunc)("error opening/reading %s: %s",
-						tzlb_filename, strerror(err));
-		    }
+		    tzfilerr(err, &ls);
 		}
 		sp->leapcnt = 0;		/* so, we're off a little */
 	}
@@ -1340,19 +1315,13 @@
 {
 	int err;
 	enum tzparseret parse_err;
-	if ((err = tzload(gmt, sp, true)) != 0) {
+	struct local_storage ls;	/* need to malloc if ALL_STATE? */
+	if ((err = tzload(gmt, sp, true, &ls)) != 0) {
 		if((parse_err = tzparse(gmt, sp, true)) != TZP_SUCCESS) {
 			if(wrnfunc) {
 				if(parse_err == TZP_PARSE_ERR)
 					(*wrnfunc)("error parsing zone string");
-				else if(*tzlb_filename) {
-					if(tzlb_reason)
-						(*wrnfunc)("%s: %s", tzlb_filename, tzlb_reason);
-					if(!tzlb_reason || err != EINVAL)
-						(*wrnfunc)("error opening/reading %s: %s",
-						 tzlb_filename, strerror(err));
-				}
-
+				else	tzfilerr(err, &ls);
 				(*wrnfunc)("can't load GMT rules %s", gmt);
 			}
 		}
@@ -1378,7 +1347,8 @@
     sp->defaulttype = 0;
     return 0;
   } else {
-    int err = tzload(name, sp, true);
+    struct local_storage ls;		/* need to malloc if ALL_STATE? */
+    int err = tzload(name, sp, true, &ls);
     int parse_err;
     if (err != 0 && name && name[0] != ':' &&
 			(parse_err = tzparse(name, sp, false)) == TZP_SUCCESS)
@@ -1389,15 +1359,8 @@
       if(wrnfunc) {
 	if(parse_err == TZP_PARSE_ERR)
 	  (*wrnfunc)("error parsing zone string");
-	else if(*tzlb_filename) {
-	  if(tzlb_reason)
-	    (*wrnfunc)("%s: %s", tzlb_filename, tzlb_reason);
-	  if(!tzlb_reason || err != EINVAL)
-	    (*wrnfunc)("error opening/reading %s: %s",
-						tzlb_filename, strerror(err));
-	} else if(tzlb_reason) {
-	    (*wrnfunc)("%s", tzlb_reason);
-	}
+	else
+	  tzfilerr(err, &ls);
 	(*wrnfunc)("can't load zone %s (using GMT instead)", name);
       }
     }
@@ -2450,3 +2413,22 @@
 {
 	wrnfunc = stderr_wrnfunc;
 }
+
+void tz_set_verbose_debug(int b)
+{
+	verbose_debug = b;
+}
+
+static void tzfilerr(int err, struct local_storage *lsp)
+{
+	if(!wrnfunc) return;
+	if(*lsp->fullname) {
+		if(lsp->failreason)
+			(*wrnfunc)("%s: %s", lsp->fullname, lsp->failreason);
+		if(!lsp->failreason || err != EINVAL)
+			(*wrnfunc)("error opening/reading %s: %s",
+						lsp->fullname, strerror(err));
+	} else if(lsp->failreason) {
+		(*wrnfunc)("%s", lsp->failreason);
+	}
+}


More information about the tz mailing list