Fix miscellaneous update-game-score bugs.

* configure.ac (difftime): Remove.
* lib/update-game-score.c (difftime) [!HAVE_DIFFTIME]: Remove.
(read_score) [HAVE_GETDELIM]: Don't access uninitialized storage.
(read_scores, write_scores): Check for fclose failure.
(write_scores): Use fchmod, not chmod, to avoid a race.
(lock_file): Fix test for out-of-date lock file; it was reversed.
Use ordinary subtraction rather than difftime; since we're already
assuming POSIX we don't need to worry about the possibility of
time_t being a magic cookie.

diff --git a/ChangeLog b/ChangeLog
index 5d82fc0b6d6da111a1bc3ddf39dc429d51971ccb..f9cec0339ce9a69e80099d02f8611af29ea8c122 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2014-01-22  Paul Eggert  <eggert@cs.ucla.edu>
+
+       Fix miscellaneous update-game-score bugs.
+       * configure.ac (difftime): Remove.
+
 2014-01-20  Paul Eggert  <eggert@cs.ucla.edu>
 
        Merge from gnulib, incorporating:

diff --git a/configure.ac b/configure.ac
index 8443d168c7dca0e137bc24d55602089ad6a475a0..626dcb1c3eaf17b5465537e8c191ff487034ed71 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -3468,7 +3468,6 @@ getrlimit setrlimit shutdown getaddrinfo \
 strsignal setitimer \
 sendto recvfrom getsockname getpeername getifaddrs freeifaddrs \
 gai_strerror getline getdelim sync \
-difftime \
 getpwent endpwent getgrent endgrent \
 touchlock \
 cfmakeraw cfsetspeed copysign __executable_start log2)

diff --git a/lib-src/ChangeLog b/lib-src/ChangeLog
index a354f832f3272ab5c3c69dd72e0d67245604086f..0d06a926bae55e9e2195aab82dbf5ff244196fdb 100644 (file)
--- a/lib-src/ChangeLog
+++ b/lib-src/ChangeLog
@@ -1,3 +1,15 @@
+2014-01-22  Paul Eggert  <eggert@cs.ucla.edu>
+
+       Fix miscellaneous update-game-score bugs.
+       * update-game-score.c (difftime) [!HAVE_DIFFTIME]: Remove.
+       (read_score) [HAVE_GETDELIM]: Don't access uninitialized storage.
+       (read_scores, write_scores): Check for fclose failure.
+       (write_scores): Use fchmod, not chmod, to avoid a race.
+       (lock_file): Fix test for out-of-date lock file; it was reversed.
+       Use ordinary subtraction rather than difftime; since we're already
+       assuming POSIX we don't need to worry about the possibility of
+       time_t being a magic cookie.
+
 2014-01-19  Paul Eggert  <eggert@cs.ucla.edu>
 
        update-game-score fixes for -m and integer overflow (Bug#16428)

diff --git a/lib-src/update-game-score.c b/lib-src/update-game-score.c
index 916a351432d7bbd5fae8c370cf49a2334a32953f..d9218ff67b784cc21940b9daddd9e66431743422 100644 (file)
--- a/lib-src/update-game-score.c
+++ b/lib-src/update-game-score.c
@@ -59,11 +59,6 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #define MAX_ATTEMPTS 5
 #define MAX_DATA_LEN 1024
 
-#ifndef HAVE_DIFFTIME
-/* OK on POSIX (time_t is arithmetic type) modulo overflow in subtraction.  */
-#define difftime(t1, t0) (double)((t1) - (t0))
-#endif
-
 static _Noreturn void
 usage (int err)
 {
@@ -275,6 +270,7 @@ read_score (FILE *f, struct score_entry *score)
 #ifdef HAVE_GETDELIM
   {
     size_t count = 0;
+    score->username = 0;
     if (getdelim (&score->username, &count, ' ', f) < 1
        || score->username == NULL)
       return -1;
@@ -371,14 +367,13 @@ read_scores (const char *filename, struct score_entry **scores,
   while ((readval = read_score (f, &entry)) == 0)
     if (push_score (&ret, &scorecount, &cursize, &entry) < 0)
       return -1;
-  if (readval > 0)
+  if (readval > 0 && fclose (f) == 0)
     {
       *count = scorecount;
       *alloc = cursize;
       *scores = ret;
       retval = 0;
     }
-  fclose (f);
   return retval;
 }
 
@@ -448,6 +443,8 @@ write_scores (const char *filename, const struct score_entry *scores,
   fd = mkostemp (tempfile, 0);
   if (fd < 0)
     return -1;
+  if (fchmod (fd, 0644) != 0)
+    return -1;
   f = fdopen (fd, "w");
   if (! f)
     return -1;
@@ -456,10 +453,9 @@ write_scores (const char *filename, const struct score_entry *scores,
                 scores[i].score, scores[i].username, scores[i].data)
        < 0)
       return -1;
-  fclose (f);
-  if (rename (tempfile, filename) < 0)
+  if (fclose (f) != 0)
     return -1;
-  if (chmod (filename, 0644) < 0)
+  if (rename (tempfile, filename) != 0)
     return -1;
   return 0;
 }
@@ -479,9 +475,9 @@ lock_file (const char *filename, void **state)
   *state = lockpath;
  trylock:
   attempts++;
-  /* If the lock is over an hour old, delete it. */
+  /* If the lock is over an hour old, delete it.  */
   if (stat (lockpath, &buf) == 0
-      && (difftime (buf.st_ctime, time (NULL) > 60*60)))
+      && 60 * 60 < time (0) - buf.st_ctime)
     unlink (lockpath);
   fd = open (lockpath, O_CREAT | O_EXCL, 0600);
   if (fd < 0)