[tz] ALL_STATE not "all" enough
Paul Eggert
eggert at cs.ucla.edu
Sun Jun 15 06:45:24 UTC 2014
enh wrote:
> we'll fix this in Android (the first hunk in
> https://android-review.googlesource.com/#/c/94653/) but it would be nice if
> this were fixed upstream too so we don't have to diverge more than we
> already have.
Thanks for mentioning this. I installed the attached patch into the
experimental repository on github. This should fix the problem you
mentioned, along with a couple of other problems of the same ilk. This
patch tries to things a bit so more-efficiently than what Android does,
by calling malloc at most once per function.
This patch assumes the buffer-overrun patch that I recently posted to
the list.
-------------- next part --------------
diff --git a/NEWS b/NEWS
index ee48de9..442f8c9 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,9 @@ Unreleased, experimental changes
A stack-overrun bug has been fixed in 'mktime' and similar functions.
+ 'localtime', 'mktime', etc. now use much less stack space if ALL_STATE
+ is defined. (Thanks to Elliott Hughes for reporting the problem.)
+
Changes affecting commentary
Commentary now uses UTF-8 instead of US-ASCII, allowing the use of
diff --git a/localtime.c b/localtime.c
index 5eecdc1..b45ed0d 100644
--- a/localtime.c
+++ b/localtime.c
@@ -335,22 +335,7 @@ tzload(register const char *name, register struct state *const sp,
2 * sizeof *sp +
4 * TZ_MAX_TIMES];
} u_t;
-#ifdef ALL_STATE
- register u_t * const up = malloc(sizeof *up);
-#else /* !defined ALL_STATE */
- u_t u;
- register u_t * const up = &u;
-#endif /* !defined ALL_STATE */
-
- sp->goback = sp->goahead = FALSE;
-
- if (up == NULL)
- return -1;
-
- if (name == NULL && (name = TZDEFAULT) == NULL)
- goto oops;
- {
- register int doaccess;
+ union local_storage {
/*
** Section 4.9.1 of the C standard says that
** "FILENAME_MAX expands to an integral constant expression
@@ -360,29 +345,56 @@ tzload(register const char *name, register struct state *const sp,
*/
char fullname[FILENAME_MAX + 1];
- if (name[0] == ':')
- ++name;
- doaccess = name[0] == '/';
- if (!doaccess) {
- if ((p = TZDIR) == NULL)
- goto oops;
- if ((strlen(p) + strlen(name) + 1) >= sizeof fullname)
- goto oops;
- (void) strcpy(fullname, p);
- (void) strcat(fullname, "/");
- (void) strcat(fullname, name);
- /*
- ** Set doaccess if '.' (as in "../") shows up in name.
- */
- if (strchr(name, '.') != NULL)
- doaccess = TRUE;
- name = fullname;
- }
- if (doaccess && access(name, R_OK) != 0)
+ /* The main part of the storage for this function. */
+ struct {
+ u_t u;
+ struct state st;
+ } u;
+ };
+ register char *fullname;
+ register u_t *up;
+ register int doaccess;
+ register union local_storage *lsp;
+#ifdef ALL_STATE
+ lsp = malloc(sizeof *lsp);
+ if (!lsp)
+ return -1;
+#else /* !defined ALL_STATE */
+ union local_storage ls;
+ lsp = &ls;
+#endif /* !defined ALL_STATE */
+ fullname = lsp->fullname;
+ up = &lsp->u.u;
+
+ sp->goback = sp->goahead = FALSE;
+
+ if (! name) {
+ name = TZDEFAULT;
+ if (! name)
goto oops;
- if ((fid = open(name, OPEN_MODE)) == -1)
+ }
+
+ if (name[0] == ':')
+ ++name;
+ doaccess = name[0] == '/';
+ if (!doaccess) {
+ p = TZDIR;
+ if (! p || sizeof lsp->fullname - 1 <= strlen(p) + strlen(name))
goto oops;
+ strcpy(fullname, p);
+ strcat(fullname, "/");
+ strcat(fullname, name);
+ /* Set doaccess if '.' (as in "../") shows up in name. */
+ if (strchr(name, '.'))
+ doaccess = TRUE;
+ name = fullname;
}
+ if (doaccess && access(name, R_OK) != 0)
+ goto oops;
+ fid = open(name, OPEN_MODE);
+ if (fid < 0)
+ goto oops;
+
nread = read(fid, up->buf, sizeof up->buf);
if (close(fid) < 0 || nread <= 0)
goto oops;
@@ -515,36 +527,36 @@ tzload(register const char *name, register struct state *const sp,
if (doextend && nread > 2 &&
up->buf[0] == '\n' && up->buf[nread - 1] == '\n' &&
sp->typecnt + 2 <= TZ_MAX_TYPES) {
- struct state ts;
+ struct state *ts = &lsp->u.st;
register int result;
up->buf[nread - 1] = '\0';
- result = tzparse(&up->buf[1], &ts, FALSE);
- if (result == 0 && ts.typecnt == 2 &&
- sp->charcnt + ts.charcnt <= TZ_MAX_CHARS) {
+ result = tzparse(&up->buf[1], ts, FALSE);
+ if (result == 0 && ts->typecnt == 2 &&
+ sp->charcnt + ts->charcnt <= TZ_MAX_CHARS) {
for (i = 0; i < 2; ++i)
- ts.ttis[i].tt_abbrind +=
+ ts->ttis[i].tt_abbrind +=
sp->charcnt;
- for (i = 0; i < ts.charcnt; ++i)
+ for (i = 0; i < ts->charcnt; ++i)
sp->chars[sp->charcnt++] =
- ts.chars[i];
+ ts->chars[i];
i = 0;
- while (i < ts.timecnt &&
- ts.ats[i] <=
+ while (i < ts->timecnt &&
+ ts->ats[i] <=
sp->ats[sp->timecnt - 1])
++i;
- while (i < ts.timecnt &&
+ while (i < ts->timecnt &&
sp->timecnt < TZ_MAX_TIMES) {
sp->ats[sp->timecnt] =
- ts.ats[i];
+ ts->ats[i];
sp->types[sp->timecnt] =
sp->typecnt +
- ts.types[i];
+ ts->types[i];
++sp->timecnt;
++i;
}
- sp->ttis[sp->typecnt++] = ts.ttis[0];
- sp->ttis[sp->typecnt++] = ts.ttis[1];
+ sp->ttis[sp->typecnt++] = ts->ttis[0];
+ sp->ttis[sp->typecnt++] = ts->ttis[1];
}
}
if (sp->timecnt > 1) {
@@ -1867,8 +1879,8 @@ time1(struct tm *const tmp,
register int sameind, otherind;
register int i;
register int nseen;
- int seen[TZ_MAX_TYPES];
- int types[TZ_MAX_TIMES];
+ char seen[TZ_MAX_TYPES];
+ unsigned char types[TZ_MAX_TIMES];
int okay;
if (tmp == NULL) {
More information about the tz
mailing list