[tz] [PATCH 4/5] Update each zic output file atomically
Paul Eggert
eggert at cs.ucla.edu
Tue Feb 23 07:05:07 UTC 2021
Without this patch, zic removed the output file and then created
its contents step by step, which meant that other processes could
encounter a zic output file that was missing or incomplete.
Now, zic writes to a temporary file and then renames it, so that
each individual output file is updated atomically.
* NEWS: Mention this.
* zic.c (S_ISDIR, itsdir, hardlinkerr): Remove; no longer needed.
(random_dirent, open_outfile, rename_dest): New functions.
(dolink): Instead of unlinking the destination, just do the link.
If that fails with EEXIST, link to a temporary file and then rename.
(writezone): Instead of unlinking the destination, write to
a temporary file and then rename.
(mkdirs): Do not bother calling itsdir, since the code
will report a reasonable error even without this check.
---
NEWS | 5 ++
zic.c | 268 ++++++++++++++++++++++++++++++++++------------------------
2 files changed, 163 insertions(+), 110 deletions(-)
diff --git a/NEWS b/NEWS
index b63d426..d559d67 100644
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,11 @@ Unreleased, experimental changes
Changes to code
+ zic now creates each output file or link atomically,
+ possibly by creating a temporary file and then renaming it.
+ This avoids races where a TZ setting would temporarily stop
+ working while zic was installing a replacement file or link.
+
Fix bug that caused 'localtime' etc. to crash when TZ was
set to a all-year DST string like "EST5EDT4,0/0,J365/25" that does
not conform to POSIX but does conform to Internet RFC 8536.
diff --git a/zic.c b/zic.c
index f7155b1..8298198 100644
--- a/zic.c
+++ b/zic.c
@@ -42,10 +42,6 @@ typedef int_fast64_t zic_t;
#else
#define MKDIR_UMASK 0755
#endif
-/* Port to native MS-Windows and to ancient UNIX. */
-#if !defined S_ISDIR && defined S_IFDIR && defined S_IFMT
-# define S_ISDIR(mode) (((mode) & S_IFMT) == S_IFDIR)
-#endif
#if HAVE_SYS_WAIT_H
#include <sys/wait.h> /* for WIFEXITED and WEXITSTATUS */
@@ -166,7 +162,6 @@ static void inrule(char ** fields, int nfields);
static bool inzcont(char ** fields, int nfields);
static bool inzone(char ** fields, int nfields);
static bool inzsub(char **, int, bool);
-static bool itsdir(char const *);
static bool itssymlink(char const *);
static bool is_alpha(char a);
static char lowerit(char);
@@ -924,6 +919,99 @@ namecheck(const char *name)
return componentcheck(name, component, cp);
}
+/* Generate a randomish name in the same directory as *NAME. If
+ *NAMEALLOC, put the name into *NAMEALLOC which is assumed to be
+ that returned by a previous call and is thus already almost set up
+ and and equal to *NAME; otherwise, allocate a new name and put its
+ address into both *NAMEALLOC and *NAME. */
+static void
+random_dirent(char const **name, char **namealloc)
+{
+ char const *src = *name;
+ char *dst = *namealloc;
+ static char const prefix[] = ".zic";
+ static char const alphabet[] = ("abcdefghijklmnopqrstuvwxyz"
+ "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+ "0123456789");
+ enum { prefixlen = sizeof prefix - 1, alphabetlen = sizeof alphabet - 1 };
+ int suffixlen = 6;
+ char const *lastslash = strrchr(src, '/');
+ ptrdiff_t dirlen = lastslash ? lastslash + 1 - src : 0;
+ static unsigned short initialized;
+ int i;
+
+ if (!dst) {
+ dst = emalloc(dirlen + prefixlen + suffixlen + 1);
+ memcpy(dst, src, dirlen);
+ memcpy(dst + dirlen, prefix, prefixlen);
+ dst[dirlen + prefixlen + suffixlen] = '\0';
+ *name = *namealloc = dst;
+ }
+
+ /* This randomization is not the best, but is portable to C89. */
+ if (!initialized++) {
+ unsigned now = time(NULL);
+ srand(rand () ^ now);
+ }
+ for (i = 0; i < suffixlen; i++)
+ dst[dirlen + prefixlen + i] = alphabet[rand () % alphabetlen];
+}
+
+/* Prepare to write to the file *OUTNAME, using *TEMPNAME to store the
+ name of the temporary file that will eventually be renamed to
+ *OUTNAME. Assign the temporary file's name to both *OUTNAME and
+ *TEMPNAME. If *TEMPNAME is null, allocate the name of any such
+ temporary file; otherwise, reuse *TEMPNAME's storage, which is
+ already set up and only needs its trailing suffix updated. */
+static FILE *
+open_outfile(char const **outname, char **tempname)
+{
+#if __STDC_VERSION__ < 201112
+ static char const fopen_mode[] = "wb";
+#else
+ static char const fopen_mode[] = "wbx";
+#endif
+
+ FILE *fp;
+ bool dirs_made = false;
+ if (!*tempname)
+ random_dirent(outname, tempname);
+
+ while (! (fp = fopen(*outname, fopen_mode))) {
+ int fopen_errno = errno;
+ if (fopen_errno == ENOENT && !dirs_made) {
+ mkdirs(*outname, true);
+ dirs_made = true;
+ } else if (fopen_errno == EEXIST)
+ random_dirent(outname, tempname);
+ else {
+ fprintf(stderr, _("%s: Can't create %s/%s: %s\n"),
+ progname, directory, *outname, strerror(fopen_errno));
+ exit(EXIT_FAILURE);
+ }
+ }
+
+ return fp;
+}
+
+/* If TEMPNAME, the result is in the temporary file TEMPNAME even
+ though the user wanted it in NAME, so rename TEMPNAME to NAME.
+ Report an error and exit if there is trouble. Also, free TEMPNAME. */
+static void
+rename_dest(char *tempname, char const *name)
+{
+ if (tempname) {
+ if (rename(tempname, name) != 0) {
+ int rename_errno = errno;
+ remove(tempname);
+ fprintf(stderr, _("%s: rename to %s/%s: %s\n"),
+ progname, directory, name, strerror(rename_errno));
+ exit(EXIT_FAILURE);
+ }
+ free(tempname);
+ }
+}
+
/* Create symlink contents suitable for symlinking FROM to TO, as a
freshly allocated string. FROM should be a relative file name, and
is relative to the global variable DIRECTORY. TO can be either
@@ -962,63 +1050,73 @@ relname(char const *target, char const *linkname)
return result;
}
-/* Hard link FROM to TO, following any symbolic links.
- Return 0 if successful, an error number otherwise. */
-static int
-hardlinkerr(char const *target, char const *linkname)
-{
- int r = linkat(AT_FDCWD, target, AT_FDCWD, linkname, AT_SYMLINK_FOLLOW);
- return r == 0 ? 0 : errno;
-}
-
static void
dolink(char const *target, char const *linkname, bool staysymlink)
{
- bool remove_only = strcmp(target, "-") == 0;
bool linkdirs_made = false;
int link_errno;
+ char *tempname = NULL;
+ char const *outname = linkname;
- /*
- ** We get to be careful here since
- ** there's a fair chance of root running us.
- */
- if (!remove_only && itsdir(target)) {
- fprintf(stderr, _("%s: linking target %s/%s failed: %s\n"),
- progname, directory, target, strerror(EPERM));
- exit(EXIT_FAILURE);
+ if (strcmp(target, "-") == 0) {
+ if (remove(linkname) == 0 || errno == ENOENT || errno == ENOTDIR)
+ return;
+ else {
+ char const *e = strerror(errno);
+ fprintf(stderr, _("%s: Can't remove %s/%s: %s\n"),
+ progname, directory, linkname, e);
+ exit(EXIT_FAILURE);
+ }
}
- if (staysymlink)
- staysymlink = itssymlink(linkname);
- if (remove(linkname) == 0)
- linkdirs_made = true;
- else if (errno != ENOENT) {
- char const *e = strerror(errno);
- fprintf(stderr, _("%s: Can't remove %s/%s: %s\n"),
- progname, directory, linkname, e);
- exit(EXIT_FAILURE);
- }
- if (remove_only)
- return;
- link_errno = staysymlink ? ENOTSUP : hardlinkerr(target, linkname);
- if (link_errno == ENOENT && !linkdirs_made) {
- mkdirs(linkname, true);
- linkdirs_made = true;
- link_errno = hardlinkerr(target, linkname);
+
+ while (true) {
+ if (linkat(AT_FDCWD, target, AT_FDCWD, outname, AT_SYMLINK_FOLLOW)
+ == 0) {
+ link_errno = 0;
+ break;
+ }
+ link_errno = errno;
+ if (link_errno == EXDEV || link_errno == ENOTSUP)
+ break;
+
+ if (link_errno == EEXIST) {
+ staysymlink &= !tempname;
+ random_dirent(&outname, &tempname);
+ if (staysymlink && itssymlink(linkname))
+ break;
+ } else if (link_errno == ENOENT && !linkdirs_made) {
+ mkdirs(linkname, true);
+ linkdirs_made = true;
+ } else {
+ fprintf(stderr, _("%s: Can't link %s/%s to %s/%s: %s\n"),
+ progname, directory, target, directory, outname,
+ strerror(link_errno));
+ exit(EXIT_FAILURE);
+ }
}
if (link_errno != 0) {
bool absolute = *target == '/';
char *linkalloc = absolute ? NULL : relname(target, linkname);
char const *contents = absolute ? target : linkalloc;
- int symlink_errno = symlink(contents, linkname) == 0 ? 0 : errno;
- if (!linkdirs_made
- && (symlink_errno == ENOENT || symlink_errno == ENOTSUP)) {
- mkdirs(linkname, true);
- if (symlink_errno == ENOENT)
- symlink_errno = symlink(contents, linkname) == 0 ? 0 : errno;
+ int symlink_errno;
+
+ while (true) {
+ if (symlink(contents, outname) == 0) {
+ symlink_errno = 0;
+ break;
+ }
+ symlink_errno = errno;
+ if (symlink_errno == EEXIST)
+ random_dirent(&outname, &tempname);
+ else if (symlink_errno == ENOENT && !linkdirs_made) {
+ mkdirs(linkname, true);
+ linkdirs_made = true;
+ } else
+ break;
}
free(linkalloc);
if (symlink_errno == 0) {
- if (link_errno != ENOTSUP)
+ if (link_errno != ENOTSUP && link_errno != EEXIST)
warning(_("symbolic link used because hard link failed: %s"),
strerror(link_errno));
} else {
@@ -1031,17 +1129,11 @@ dolink(char const *target, char const *linkname, bool staysymlink)
progname, directory, target, e);
exit(EXIT_FAILURE);
}
- tp = fopen(linkname, "wb");
- if (!tp) {
- char const *e = strerror(errno);
- fprintf(stderr, _("%s: Can't create %s/%s: %s\n"),
- progname, directory, linkname, e);
- exit(EXIT_FAILURE);
- }
+ tp = open_outfile(&outname, &tempname);
while ((c = getc(fp)) != EOF)
putc(c, tp);
close_file(fp, directory, target);
- close_file(tp, directory, linkname);
+ close_file(tp, directory, outname);
if (link_errno != ENOTSUP)
warning(_("copy used because hard link failed: %s"),
strerror(link_errno));
@@ -1050,29 +1142,7 @@ dolink(char const *target, char const *linkname, bool staysymlink)
strerror(symlink_errno));
}
}
-}
-
-/* Return true if NAME is a directory. */
-static bool
-itsdir(char const *name)
-{
- struct stat st;
- int res = stat(name, &st);
-#ifdef S_ISDIR
- if (res == 0)
- return S_ISDIR(st.st_mode) != 0;
-#endif
- if (res == 0 || errno == EOVERFLOW) {
- size_t n = strlen(name);
- char *nameslashdot = emalloc(n + 3);
- bool dir;
- memcpy(nameslashdot, name, n);
- strcpy(&nameslashdot[n], &"/."[! (n && name[n - 1] != '/')]);
- dir = stat(nameslashdot, &st) == 0 || errno == EOVERFLOW;
- free(nameslashdot);
- return dir;
- }
- return false;
+ rename_dest(tempname, linkname);
}
/* Return true if NAME is a symbolic link. */
@@ -1868,10 +1938,11 @@ writezone(const char *const name, const char *const string, char version,
register FILE * fp;
register ptrdiff_t i, j;
register int pass;
- bool dir_checked = false;
zic_t one = 1;
zic_t y2038_boundary = one << 31;
ptrdiff_t nats = timecnt + WORK_AROUND_QTBUG_53071;
+ char *tempname = NULL;
+ char const *outname = name;
/* Allocate the ATS and TYPES arrays via a single malloc,
as this is a bit faster. */
@@ -1969,32 +2040,8 @@ writezone(const char *const name, const char *const string, char version,
range64 = limitrange(rangeall, lo_time, hi_time, ats, types);
range32 = limitrange(range64, INT32_MIN, INT32_MAX, ats, types);
- /*
- ** Remove old file, if any, to snap links.
- */
- if (remove(name) == 0)
- dir_checked = true;
- else if (errno != ENOENT) {
- const char *e = strerror(errno);
+ fp = open_outfile(&outname, &tempname);
- fprintf(stderr, _("%s: Can't remove %s/%s: %s\n"),
- progname, directory, name, e);
- exit(EXIT_FAILURE);
- }
- fp = fopen(name, "wb");
- if (!fp) {
- int fopen_errno = errno;
- if (fopen_errno == ENOENT && !dir_checked) {
- mkdirs(name, true);
- fp = fopen(name, "wb");
- fopen_errno = errno;
- }
- if (!fp) {
- fprintf(stderr, _("%s: Can't create %s/%s: %s\n"),
- progname, directory, name, strerror(fopen_errno));
- exit(EXIT_FAILURE);
- }
- }
for (pass = 1; pass <= 2; ++pass) {
register ptrdiff_t thistimei, thistimecnt, thistimelim;
register int thisleapi, thisleapcnt, thisleaplim;
@@ -2263,7 +2310,8 @@ writezone(const char *const name, const char *const string, char version,
putc(ttisuts[i], fp);
}
fprintf(fp, "\n%s\n", string);
- close_file(fp, directory, name);
+ close_file(fp, directory, outname);
+ rename_dest(tempname, name);
free(ats);
}
@@ -3393,7 +3441,7 @@ mp = _("time zone abbreviation differs from POSIX standard");
/* Ensure that the directories of ARGNAME exist, by making any missing
ones. If ANCESTORS, do this only for ARGNAME's ancestors; otherwise,
do it for ARGNAME too. Exit with failure if there is trouble.
- Do not consider an existing non-directory to be trouble. */
+ Do not consider an existing file to be trouble. */
static void
mkdirs(char const *argname, bool ancestors)
{
@@ -3424,11 +3472,11 @@ mkdirs(char const *argname, bool ancestors)
** is checked anyway if the mkdir fails.
*/
if (mkdir(name, MKDIR_UMASK) != 0) {
- /* For speed, skip itsdir if errno == EEXIST. Since
- mkdirs is called only after open fails with ENOENT
- on a subfile, EEXIST implies itsdir here. */
+ /* Do not report an error if err == EEXIST, because
+ some other process might have made the directory
+ in the meantime. */
int err = errno;
- if (err != EEXIST && !itsdir(name)) {
+ if (err != EEXIST) {
error(_("%s: Can't create directory %s: %s"),
progname, name, strerror(err));
exit(EXIT_FAILURE);
--
2.27.0
More information about the tz
mailing list