This avoids some problems in creating nonworking hard links to symlinks, as well as removing the need for some stat calls. * NEWS: Mention this. * zic.c (lstat, S_ISLNK) [!HAVE_SYMLINK]: New fallback macros. (dolink): Remove destination before linking; this is more consistent with what is done with fopen, and simplifies the code. It also lets us avoid some mkdir calls in some cases. (itsdir): Document that errno is set when -1 is returned. Use lstat, not stat. Avoid tricky code with EOVERFLOW, as it doesn't work with lstat and probably wasn't worth the trouble anyway. (writezone): Do not bother statting the destination before removing it. The old code catered to ancient platforms that lacked the 'remove' function and where trouble could ensue if a program unlinked a directory. We don't need to worry about these platforms any more, as the code has been successfully using 'remove' since the 1980s. (mkdirs): Exit on failure, and return true if some directories are made. All callers changed. Allow symlinks as well as directories. --- NEWS | 3 ++- zic.c | 98 ++++++++++++++++++++++++++++++++++++------------------------------- 2 files changed, 54 insertions(+), 47 deletions(-) diff --git a/NEWS b/NEWS index 192deb7..a2a57fc 100644 --- a/NEWS +++ b/NEWS @@ -55,7 +55,8 @@ Unreleased, experimental changes stamps on the reference platform. (Thanks to Alexander Belopolsky for reporting the bug and suggesting a way forward.) - zic now avoids some unnecessary mkdir and stat system calls. + zic now avoids hard linking to symbolic links, and avoids some + unnecessary mkdir and stat system calls. zdump has a new -i option to generate transitions in a more-compact but still human-readable format. This option is diff --git a/zic.c b/zic.c index 1b7124a..1c8a193 100644 --- a/zic.c +++ b/zic.c @@ -106,7 +106,9 @@ extern int optind; # define link(from, to) (errno = ENOTSUP, -1) #endif #if ! HAVE_SYMLINK +# define lstat(name, st) stat(name, st) # define symlink(from, to) (errno = ENOTSUP, -1) +# define S_ISLNK(m) 0 #endif static void addtt(zic_t starttime, int type); @@ -762,6 +764,7 @@ dolink(char const *fromfield, char const *tofield) register char * fromname; register char * toname; register int fromisdir; + int todirs_made = -1; fromname = relname(directory, fromfield); toname = relname(directory, tofield); @@ -776,23 +779,21 @@ dolink(char const *fromfield, char const *tofield) progname, fromname, e); exit(EXIT_FAILURE); } + if (remove(toname) == 0) + todirs_made = 0; + else if (errno != ENOENT) { + char const *e = strerror(errno); + fprintf(stderr, _("%s: Can't remove %s: %s\n"), progname, toname, e); + exit(EXIT_FAILURE); + } if (link(fromname, toname) != 0) { int link_errno = errno; - bool todirs_made = false; - bool retry_if_link_supported = false; - - if (link_errno == ENOENT) { - if (! mkdirs(toname)) - exit(EXIT_FAILURE); - todirs_made = true; - retry_if_link_supported = true; + + if (link_errno == ENOENT && todirs_made < 0) { + todirs_made = mkdirs(toname); + if (todirs_made) + link_errno = link(fromname, toname) == 0 ? 0 : errno; } - if ((link_errno == EEXIST || link_errno == ENOTSUP) - && itsdir(toname) == 0 - && (remove(toname) == 0 || errno == ENOENT)) - retry_if_link_supported = true; - if (retry_if_link_supported && link_errno != ENOTSUP) - link_errno = link(fromname, toname) == 0 ? 0 : errno; if (link_errno != 0) { const char *s = fromfield; const char *t; @@ -813,11 +814,9 @@ dolink(char const *fromfield, char const *tofield) memcpy(p, "../", 3); strcpy(p, t); symlink_result = symlink(symlinkcontents, toname); - if (!todirs_made && symlink_result != 0 && errno == ENOENT) { - if (! mkdirs(toname)) - exit(EXIT_FAILURE); + if (symlink_result != 0 && errno == ENOENT + && todirs_made < 0 && mkdirs(toname)) symlink_result = symlink(symlinkcontents, toname); - } free(symlinkcontents); if (symlink_result == 0) { if (link_errno != ENOTSUP) @@ -899,21 +898,22 @@ static const zic_t early_time = (WORK_AROUND_GNOME_BUG_730332 ? BIG_BANG : MINVAL(zic_t, TIME_T_BITS_IN_FILE)); -/* Return 1 if NAME is a directory, 0 if it's something else, -1 if trouble. */ +/* Return 1 if NAME is a directory or a symbolic link, 0 if it's + something else, -1 (setting errno) if trouble. */ static int itsdir(char const *name) { struct stat st; - int res = stat(name, &st); + int res = lstat(name, &st); + if (res == 0) { #ifdef S_ISDIR - if (res == 0) - return S_ISDIR(st.st_mode) != 0; -#endif - if (res == 0 || errno == EOVERFLOW) { + return S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode); +#else char *nameslashdot = relname(name, "."); - bool dir = stat(nameslashdot, &st) == 0 || errno == EOVERFLOW; + bool dir = lstat(nameslashdot, &st) == 0; free(nameslashdot); return dir; +#endif } return -1; } @@ -1640,6 +1640,7 @@ writezone(const char *const name, const char *const string, char version) char * fullname; static const struct tzhead tzh0; static struct tzhead tzh; + bool dir_checked = false; zic_t one = 1; zic_t y2038_boundary = one << 31; int nats = timecnt + WORK_AROUND_QTBUG_53071; @@ -1743,7 +1744,9 @@ writezone(const char *const name, const char *const string, char version) /* ** Remove old file, if any, to snap links. */ - if (itsdir(fullname) == 0 && remove(fullname) != 0 && errno != ENOENT) { + if (remove(fullname) == 0) + dir_checked = true; + else if (errno != ENOENT) { const char *e = strerror(errno); fprintf(stderr, _("%s: Can't remove %s: %s\n"), @@ -1751,17 +1754,18 @@ writezone(const char *const name, const char *const string, char version) exit(EXIT_FAILURE); } fp = fopen(fullname, "wb"); - if (!fp && errno == ENOENT) { - if (! mkdirs(fullname)) - exit(EXIT_FAILURE); - fp = fopen(fullname, "wb"); - if (!fp) { - const char *e = strerror(errno); - - fprintf(stderr, _("%s: Can't create %s: %s\n"), - progname, fullname, e); - exit(EXIT_FAILURE); - } + if (!fp) { + int fopen_errno = errno; + if (fopen_errno == ENOENT && !dir_checked && mkdirs(fullname)) { + fp = fopen(fullname, "wb"); + fopen_errno = errno; + } + if (!fp) { + char const *e = strerror(fopen_errno); + fprintf(stderr, _("%s: Can't create %s: %s\n"), + progname, fullname, e); + exit(EXIT_FAILURE); + } } for (pass = 1; pass <= 2; ++pass) { register int thistimei, thistimecnt; @@ -3027,14 +3031,17 @@ mp = _("time zone abbreviation differs from POSIX standard"); charcnt += i; } +/* Ensure that the parent directories of ARGNAME exist, by making any + missing ones. Return true if some directories are made (perhaps by + some other process), false if the directories already exist, and + exit with failure if there is trouble. */ static bool mkdirs(char *argname) { register char * name; register char * cp; + bool dirs_made = false; - if (argname == NULL || *argname == '\0') - return true; cp = name = ecpyalloc(argname); /* Do not mkdir a root directory, as it must exist. */ @@ -3056,17 +3063,16 @@ mkdirs(char *argname) */ if (mkdir(name, MKDIR_UMASK) != 0) { int err = errno; - if (err != EEXIST && itsdir(name) <= 0) { + if (err != EEXIST && itsdir(name) < 0) { char const *e = strerror(err); - warning(_("%s: Can't create directory" - " %s: %s"), - progname, name, e); - free(name); - return false; + error(_("%s: Can't create directory %s: %s"), + progname, name, e); + exit(EXIT_FAILURE); } } *cp = '/'; + dirs_made = true; } free(name); - return true; + return dirs_made; } -- 2.7.4

The main reason for this is to simplify debugging. It also improves performance slightly, I suppose. * NEWS: Document this. * zic.c (close_file): New arg DIR. All callers changed. (change_directory): New function. (relname): Remove. (dolink, writezone): Simplify by assuming chdir. (itsdir) [!S_ISDIR]: Append "." by hand. (mkdirs): First arg is now a const pointer, since the storage is not modified. New arg ANCESTORS. Return void, not bool. All callers changed. --- NEWS | 5 +- zic.c | 167 +++++++++++++++++++++++++++++++++--------------------------------- 2 files changed, 87 insertions(+), 85 deletions(-) diff --git a/NEWS b/NEWS index 7f92e17..d7ba940 100644 --- a/NEWS +++ b/NEWS @@ -60,8 +60,9 @@ Unreleased, experimental changes compatibility with platforms like OpenSUSE where other programs configure these files as symlinks. - zic now avoids hard linking to symbolic links, and avoids some - unnecessary mkdir and stat system calls. + zic now avoids hard linking to symbolic links, avoids some + unnecessary mkdir and stat system calls, and uses shorter file + names internally. zdump has a new -i option to generate transitions in a more-compact but still human-readable format. This option is diff --git a/zic.c b/zic.c index 011b3cc..2505c11 100644 --- a/zic.c +++ b/zic.c @@ -130,7 +130,7 @@ static bool inzsub(char **, int, bool); static int itsdir(const char * name); static bool is_alpha(char a); static char lowerit(char); -static bool mkdirs(char *); +static void mkdirs(char const *, bool); static void newabbr(const char * abbr); static zic_t oadd(zic_t t1, zic_t t2); static void outzone(const struct zone * zp, int ntzones); @@ -498,15 +498,15 @@ warning(const char *const string, ...) } static void -close_file(FILE *stream, char const *name) +close_file(FILE *stream, char const *dir, char const *name) { char const *e = (ferror(stream) ? _("I/O error") : fclose(stream) != 0 ? strerror(errno) : NULL); if (e) { - fprintf(stderr, "%s: ", progname); - if (name) - fprintf(stderr, "%s: ", name); - fprintf(stderr, "%s\n", e); + fprintf(stderr, "%s: %s%s%s%s%s\n", progname, + dir ? dir : "", dir ? "/" : "", + name ? name : "", name ? ": " : "", + e); exit(EXIT_FAILURE); } } @@ -521,10 +521,30 @@ usage(FILE *stream, int status) "Report bugs to %s.\n"), progname, progname, REPORT_BUGS_TO); if (status == EXIT_SUCCESS) - close_file(stream, NULL); + close_file(stream, NULL, NULL); exit(status); } +/* Change the working directory to DIR, possibly creating DIR and its + ancestors. After this is done, all files are accessed with names + relative to DIR. */ +static void +change_directory (char const *dir) +{ + if (chdir(dir) != 0) { + int chdir_errno = errno; + if (chdir_errno == ENOENT) { + mkdirs(dir, false); + chdir_errno = chdir(dir) == 0 ? 0 : errno; + } + if (chdir_errno != 0) { + fprintf(stderr, _("%s: Can't chdir to %s: %s\n"), + progname, dir, strerror(chdir_errno)); + exit(EXIT_FAILURE); + } + } +} + static const char * psxrules; static const char * lcltime; static const char * directory; @@ -557,7 +577,7 @@ main(int argc, char **argv) for (i = 1; i < argc; ++i) if (strcmp(argv[i], "--version") == 0) { printf("zic %s%s\n", PKGVERSION, TZVERSION); - close_file(stdout, NULL); + close_file(stdout, NULL, NULL); return EXIT_SUCCESS; } else if (strcmp(argv[i], "--help") == 0) { usage(stdout, EXIT_SUCCESS); @@ -640,6 +660,7 @@ _("%s: More than one -L option specified\n"), if (errors) return EXIT_FAILURE; associate(); + change_directory(directory); for (i = 0; i < nzones; i = j) { /* ** Find the next non-continuation zone entry. @@ -743,58 +764,40 @@ namecheck(const char *name) return componentcheck(name, component, cp); } -static char * -relname(char const *dir, char const *base) -{ - if (*base == '/') - return ecpyalloc(base); - else { - size_t dir_len = strlen(dir); - bool needs_slash = dir_len && dir[dir_len - 1] != '/'; - char *result = emalloc(dir_len + needs_slash + strlen(base) + 1); - result[dir_len] = '/'; - strcpy(result + dir_len + needs_slash, base); - return memcpy(result, dir, dir_len); - } -} - static void dolink(char const *fromfield, char const *tofield, bool staysymlink) { - register char * fromname; - register char * toname; register int fromisdir; - int todirs_made = -1; + bool todirs_made = false; int link_errno; - fromname = relname(directory, fromfield); - toname = relname(directory, tofield); /* ** We get to be careful here since ** there's a fair chance of root running us. */ - fromisdir = itsdir(fromname); + fromisdir = itsdir(fromfield); if (fromisdir) { char const *e = strerror(fromisdir < 0 ? errno : EPERM); - fprintf(stderr, _("%s: link from %s failed: %s\n"), - progname, fromname, e); + fprintf(stderr, _("%s: link from %s/%s failed: %s\n"), + progname, directory, fromfield, e); exit(EXIT_FAILURE); } if (staysymlink) - staysymlink = itsdir(toname) == 2; - if (remove(toname) == 0) - todirs_made = 0; + staysymlink = itsdir(tofield) == 2; + if (remove(tofield) == 0) + todirs_made = true; else if (errno != ENOENT) { char const *e = strerror(errno); - fprintf(stderr, _("%s: Can't remove %s: %s\n"), progname, toname, e); + fprintf(stderr, _("%s: Can't remove %s/%s: %s\n"), + progname, directory, tofield, e); exit(EXIT_FAILURE); } link_errno = (staysymlink ? ENOTSUP - : link(fromname, toname) == 0 ? 0 : errno); - if (link_errno == ENOENT && todirs_made < 0) { - todirs_made = mkdirs(toname); - if (todirs_made) - link_errno = link(fromname, toname) == 0 ? 0 : errno; + : link(fromfield, tofield) == 0 ? 0 : errno); + if (link_errno == ENOENT && !todirs_made) { + mkdirs(tofield, true); + todirs_made = true; + link_errno = link(fromfield, tofield) == 0 ? 0 : errno; } if (link_errno != 0) { const char *s = fromfield; @@ -815,9 +818,11 @@ dolink(char const *fromfield, char const *tofield, bool staysymlink) for (p = symlinkcontents; dotdots-- != 0; p += 3) memcpy(p, "../", 3); strcpy(p, t); - symlink_errno = symlink(symlinkcontents, toname) == 0 ? 0 : errno; - if (symlink_errno == ENOENT && todirs_made < 0 && mkdirs(toname)) - symlink_errno = symlink(symlinkcontents, toname) == 0 ? 0 : errno; + symlink_errno = symlink(symlinkcontents, tofield) == 0 ? 0 : errno; + if (symlink_errno == ENOENT && !todirs_made) { + mkdirs(tofield, true); + symlink_errno = symlink(symlinkcontents, tofield) == 0 ? 0 : errno; + } free(symlinkcontents); if (symlink_errno == 0) { if (link_errno != ENOTSUP) @@ -826,24 +831,24 @@ dolink(char const *fromfield, char const *tofield, bool staysymlink) } else { FILE *fp, *tp; int c; - fp = fopen(fromname, "rb"); + fp = fopen(fromfield, "rb"); if (!fp) { char const *e = strerror(errno); - fprintf(stderr, _("%s: Can't read %s: %s\n"), - progname, fromname, e); + fprintf(stderr, _("%s: Can't read %s/%s: %s\n"), + progname, directory, fromfield, e); exit(EXIT_FAILURE); } - tp = fopen(toname, "wb"); + tp = fopen(tofield, "wb"); if (!tp) { char const *e = strerror(errno); - fprintf(stderr, _("%s: Can't create %s: %s\n"), - progname, toname, e); + fprintf(stderr, _("%s: Can't create %s/%s: %s\n"), + progname, directory, tofield, e); exit(EXIT_FAILURE); } while ((c = getc(fp)) != EOF) putc(c, tp); - close_file(fp, fromname); - close_file(tp, toname); + close_file(fp, directory, fromfield); + close_file(tp, directory, tofield); if (link_errno != ENOTSUP) warning(_("copy used because hard link failed: %s"), strerror(link_errno)); @@ -852,8 +857,6 @@ dolink(char const *fromfield, char const *tofield, bool staysymlink) strerror(symlink_errno)); } } - free(fromname); - free(toname); } #define TIME_T_BITS_IN_FILE 64 @@ -910,8 +913,12 @@ itsdir(char const *name) #ifdef S_ISDIR return S_ISDIR(st.st_mode) ? 1 : S_ISLNK(st.st_mode) ? 2 : 0; #else - char *nameslashdot = relname(name, "."); - bool dir = lstat(nameslashdot, &st) == 0; + size_t n = strlen(name); + char *nameslashdot = emalloc(n + 3); + bool dir; + memcpy(nameslashdot, name, n); + strcpy(&nameslashdot[n], &"/."[! (n && name[n - 1] != '/')]); + dir = lstat(nameslashdot, &st) == 0; free(nameslashdot); return dir; #endif @@ -1088,7 +1095,7 @@ _("%s: panic: Invalid l_value %d\n"), } free(fields); } - close_file(fp, filename); + close_file(fp, NULL, filename); if (wantcont) error(_("expected continuation line not found")); } @@ -1638,7 +1645,6 @@ writezone(const char *const name, const char *const string, char version) register int leapcnt32, leapi32; register int timecnt32, timei32; register int pass; - char * fullname; static const struct tzhead tzh0; static struct tzhead tzh; bool dir_checked = false; @@ -1741,30 +1747,29 @@ writezone(const char *const name, const char *const string, char version) --leapcnt32; ++leapi32; } - fullname = relname(directory, name); /* ** Remove old file, if any, to snap links. */ - if (remove(fullname) == 0) + if (remove(name) == 0) dir_checked = true; else if (errno != ENOENT) { const char *e = strerror(errno); - fprintf(stderr, _("%s: Can't remove %s: %s\n"), - progname, fullname, e); + fprintf(stderr, _("%s: Can't remove %s/%s: %s\n"), + progname, directory, name, e); exit(EXIT_FAILURE); } - fp = fopen(fullname, "wb"); + fp = fopen(name, "wb"); if (!fp) { int fopen_errno = errno; - if (fopen_errno == ENOENT && !dir_checked && mkdirs(fullname)) { - fp = fopen(fullname, "wb"); + if (fopen_errno == ENOENT && !dir_checked) { + mkdirs(name, true); + fp = fopen(name, "wb"); fopen_errno = errno; } if (!fp) { - char const *e = strerror(fopen_errno); - fprintf(stderr, _("%s: Can't create %s: %s\n"), - progname, fullname, e); + fprintf(stderr, _("%s: Can't create %s/%s: %s\n"), + progname, directory, name, strerror(fopen_errno)); exit(EXIT_FAILURE); } } @@ -1959,9 +1964,8 @@ writezone(const char *const name, const char *const string, char version) putc(ttisgmts[i], fp); } fprintf(fp, "\n%s\n", string); - close_file(fp, fullname); + close_file(fp, directory, name); free(ats); - free(fullname); } static char const * @@ -3032,16 +3036,14 @@ mp = _("time zone abbreviation differs from POSIX standard"); charcnt += i; } -/* Ensure that the parent directories of ARGNAME exist, by making any - missing ones. Return true if some directories are made (perhaps by - some other process), false if the directories already exist, and - exit with failure if there is trouble. */ -static bool -mkdirs(char *argname) +/* 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. */ +static void +mkdirs(char const *argname, bool ancestors) { register char * name; register char * cp; - bool dirs_made = false; cp = name = ecpyalloc(argname); @@ -3053,8 +3055,9 @@ mkdirs(char *argname) while (*cp == '/') cp++; - for (; (cp = strchr(cp, '/')) != 0; cp++) { - *cp = '\0'; + while (cp && ((cp = strchr(cp, '/')) || !ancestors)) { + if (cp) + *cp = '\0'; /* ** Try to create it. It's OK if creation fails because ** the directory already exists, perhaps because some @@ -3065,15 +3068,13 @@ mkdirs(char *argname) if (mkdir(name, MKDIR_UMASK) != 0) { int err = errno; if (err != EEXIST && itsdir(name) < 0) { - char const *e = strerror(err); error(_("%s: Can't create directory %s: %s"), - progname, name, e); + progname, name, strerror(err)); exit(EXIT_FAILURE); } } - *cp = '/'; - dirs_made = true; + if (cp) + *cp++ = '/'; } free(name); - return dirs_made; } -- 2.7.4