From f7b2570e158e8fbf84c8a96bced4f9866feeb753 Mon Sep 17 00:00:00 2001 From: Wayne Davison Date: Mon, 5 May 2008 01:14:04 +0000 Subject: Locking simplification and signed/unsigned fixes. --- ChangeLog | 11 +++- Src/Zle/zle_hist.c | 4 +- Src/Zle/zle_move.c | 4 +- Src/hist.c | 149 ++++++++++++++++++++--------------------------------- 4 files changed, 68 insertions(+), 100 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1e69df02a..e5e66fc42 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2008-05-04 Wayne Davison + + * 24831 plus changes: Src/hist.c: simplified fcntl() locking. + + * unposted: Src/Zle/zle_hist.c, Src/Zle/zle_misc.c: fixed + signed/unsigned warnings, including an always true "if". + 2008-05-04 Peter Stephenson * Phil Pennock: 24904 + 24916: configure.ac, Src/system.h: don't @@ -5,7 +12,7 @@ if the standard says we need it for wcwidth(). * 24915: Src/Zle/zle_hist.c, Src/Zle/zle_misc.c, - Src/Zle/zle_move.c: Src/Zle/zle_vi.c: some more fix-ups for + Src/Zle/zle_move.c, Src/Zle/zle_vi.c: some more fix-ups for combining characters. 2008-05-02 Peter Stephenson @@ -195,7 +202,7 @@ * unposted: NEWS: list new features so far since 4.3.6. * Vincent Lefevre: 24818: Doc/Zsh/options.yo (modified), - Src/hist.c (with #ifdef HAVE_TRUNCATE), Src/options.c, Src/zsh.h: + Src/hist.c (with #ifdef HAVE_FTRUNCATE), Src/options.c, Src/zsh.h: history file locking using fcntl(). 2008-04-16 Clint Adams diff --git a/Src/Zle/zle_hist.c b/Src/Zle/zle_hist.c index 0a4ead788..54c103f60 100644 --- a/Src/Zle/zle_hist.c +++ b/Src/Zle/zle_hist.c @@ -1316,7 +1316,7 @@ doisearch(char **args, int dir, int pattern) zlemetaline + pos, NULL) - zlemetaline; } else if (sbuf[0] != '^') { - if (pos >= strlen(zt) - 1) + if (pos >= (int)strlen(zt) - 1) skip_line = 1; else pos += 1; @@ -1598,7 +1598,7 @@ doisearch(char **args, int dir, int pattern) zlemetacs, sbptr, dir, nomatch); if (sbptr >= sibuf - FIRST_SEARCH_CHAR - 2 #ifdef MULTIBYTE_SUPPORT - - 2 * MB_CUR_MAX + - 2 * (int)MB_CUR_MAX #endif ) { ibuf = hrealloc(ibuf, sibuf, sibuf * 2); diff --git a/Src/Zle/zle_move.c b/Src/Zle/zle_move.c index 5b02616a2..f15b114f5 100644 --- a/Src/Zle/zle_move.c +++ b/Src/Zle/zle_move.c @@ -209,8 +209,8 @@ backwardmetafiedchar(char *start, char *endptr, convchar_t *retchr) return ptr; } } - if (ret >= 0) { - if (ret < charlen) { + if (ret != (size_t)-1) { + if (ret < (size_t)charlen) { /* The last character didn't convert, so use it raw. */ break; } diff --git a/Src/hist.c b/Src/hist.c index e7f211dd5..e5de9572e 100644 --- a/Src/hist.c +++ b/Src/hist.c @@ -2023,20 +2023,6 @@ readhistfile(char *fn, int err, int readflags) else if (!lockhistfile(fn, 1)) return; if ((in = fopen(unmeta(fn), "r"))) { -#ifdef HAVE_FCNTL_H - if (isset(HISTFCNTLLOCK)) { - struct flock lck; - - lck.l_type = F_RDLCK; - lck.l_whence = SEEK_SET; - lck.l_start = 0; - lck.l_len = 0; /* lock the whole file */ - if (fcntl(fileno(in), F_SETLKW, &lck) == -1) { - fclose(in); - return; - } - } -#endif nwordlist = 64; wordlist = (short *)zalloc(nwordlist*sizeof(short)); bufsiz = 1024; @@ -2166,66 +2152,34 @@ readhistfile(char *fn, int err, int readflags) } #ifdef HAVE_FCNTL_H -/**/ +static int flock_fd = -1; + static int -wlockfile(int fd) +flockhistfile(char *fn, int keep_trying) { struct flock lck; - int ctr = 8; + int ctr = keep_trying ? 9 : 0; + + if ((flock_fd = open(unmeta(fn), O_RDWR | O_NOCTTY)) < 0) + return errno == ENOENT; /* "successfully" locked missing file */ lck.l_type = F_WRLCK; lck.l_whence = SEEK_SET; lck.l_start = 0; - lck.l_len = 0; - while (fcntl(fd, F_SETLKW, &lck) == -1) { - if (--ctr < 0) - return 1; - sleep (1); - } - return 0; -} -#endif + lck.l_len = 0; /* lock the whole file */ -/**/ -static int -safe_unlink(const char *pathname) -{ -#ifdef HAVE_FCNTL_H - if (isset(HISTFCNTLLOCK)) { - int fd = open(pathname, O_WRONLY | O_NOCTTY, 0600); - if (fd >= 0) { - int err = wlockfile(fd) || unlink(pathname); - close(fd); - return err; - } else { - return errno != ENOENT; + while (fcntl(flock_fd, F_SETLKW, &lck) == -1) { + if (--ctr < 0) { + close(flock_fd); + flock_fd = -1; + return 0; } + sleep(1); } -#endif - return unlink(pathname) && errno != ENOENT; -} -/**/ -static int -safe_rename(const char *oldpath, const char *newpath) -{ -#ifdef HAVE_FCNTL_H - if (isset(HISTFCNTLLOCK)) { - int fd = open(newpath, O_CREAT | O_WRONLY | O_NOCTTY, 0600); - if (fd < 0) { - return 1; - } else if (wlockfile(fd)) { - close(fd); - return 1; - } else { - int err = rename(oldpath, newpath); - close(fd); - return err; - } - } -#endif - return rename(oldpath, newpath); + return 1; } +#endif /**/ void @@ -2236,9 +2190,6 @@ savehistfile(char *fn, int err, int writeflags) Histent he; zlong xcurhist = curhist - !!(histactive & HA_ACTIVE); int extended_history = isset(EXTENDEDHISTORY); -#ifdef HAVE_FTRUNCATE - int truncate_history = 0; -#endif int ret; if (!interact || savehistsiz <= 0 || !hist_ring @@ -2277,16 +2228,12 @@ savehistfile(char *fn, int err, int writeflags) tmpfile = NULL; out = fd >= 0 ? fdopen(fd, "a") : NULL; } else if (!isset(HISTSAVEBYCOPY)) { - int fd = open(unmeta(fn), O_CREAT | O_WRONLY | O_NOCTTY, 0600); + int fd = open(unmeta(fn), O_CREAT | O_WRONLY | O_TRUNC | O_NOCTTY, 0600); tmpfile = NULL; out = fd >= 0 ? fdopen(fd, "w") : NULL; -#ifdef HAVE_FTRUNCATE - /* The file should be truncated after its locking. */ - truncate_history = 1; -#endif } else { tmpfile = bicat(unmeta(fn), ".new"); - if (safe_unlink(tmpfile)) + if (unlink(tmpfile) < 0 && errno != ENOENT) out = NULL; else { struct stat sb; @@ -2324,18 +2271,7 @@ savehistfile(char *fn, int err, int writeflags) #endif } } -#ifdef HAVE_FCNTL_H - if (out && isset(HISTFCNTLLOCK) && wlockfile(fileno(out))) { - zerr("can't lock file (timeout) -- history %s not updated", fn); - err = 0; /* Don't report a generic error below. */ - out = NULL; - } -#endif if (out) { -#ifdef HAVE_FTRUNCATE - if (truncate_history) - ftruncate(fileno(out), 0); -#endif ret = 0; for (; he && he->histnum <= xcurhist; he = down_histent(he)) { if ((writeflags & HFILE_SKIPDUPS && he->node.flags & HIST_DUP) @@ -2381,21 +2317,26 @@ savehistfile(char *fn, int err, int writeflags) zsfree(lasthist.text); lasthist.text = ztrdup(start); } - } else if (ret >= 0 && fflush(out) < 0) { - ret = -1; } + if (fclose(out) < 0 && ret >= 0) + ret = -1; if (ret >= 0) { if (tmpfile) { - /* out has been flushed and the file must be renamed while - being open so that the lock is still valid */ - if (safe_rename(tmpfile, unmeta(fn))) + if (rename(tmpfile, unmeta(fn)) < 0) { zerr("can't rename %s.new to $HISTFILE", fn); - free(tmpfile); - tmpfile = NULL; + ret = -1; + err = 0; +#ifdef HAVE_FCNTL_H + } else { + /* We renamed over the locked HISTFILE, so close fd. + * If we do more writing, we'll get a lock then. */ + close(flock_fd); + flock_fd = -1; +#endif + } } - fclose(out); - if (writeflags & HFILE_SKIPOLD + if (ret >= 0 && writeflags & HFILE_SKIPOLD && !(writeflags & (HFILE_FAST | HFILE_NO_REWRITE))) { int remember_histactive = histactive; @@ -2413,8 +2354,6 @@ savehistfile(char *fn, int err, int writeflags) pophiststack(); histactive = remember_histactive; } - } else { - fclose(out); } } else ret = -1; @@ -2441,6 +2380,12 @@ lockhistfile(char *fn, int keep_trying) if (!fn && !(fn = getsparam("HISTFILE"))) return 0; + +#ifdef HAVE_FCNTL_H + if (isset(HISTFCNTLLOCK) && flock_fd < 0 && !flockhistfile(fn, keep_trying)) + return 0; +#endif + if (!lockhistct++) { struct stat sb; int fd; @@ -2505,7 +2450,17 @@ lockhistfile(char *fn, int keep_trying) #endif /* not HAVE_LINK */ free(lockfile); } - return ct != lockhistct; + + if (ct == lockhistct) { +#ifdef HAVE_FCNTL_H + if (flock_fd >= 0) { + close(flock_fd); + flock_fd = -1; + } +#endif + return 0; + } + return 1; } /* Unlock the history file if this corresponds to the last nested lock @@ -2529,6 +2484,12 @@ unlockhistfile(char *fn) sprintf(lockfile, "%s.LOCK", fn); unlink(lockfile); free(lockfile); +#ifdef HAVE_FCNTL_H + if (flock_fd >= 0) { + close(flock_fd); + flock_fd = -1; + } +#endif } } -- cgit 1.4.1