8a8cfb
commit 5a3afa9738f3dbbaf8c0a35665318c1af782111b
8a8cfb
Author: Florian Weimer <fweimer@redhat.com>
8a8cfb
Date:   Tue Aug 13 15:53:19 2019 +0200
8a8cfb
8a8cfb
    login: Replace macro-based control flow with function calls in utmp
8a8cfb
8a8cfb
diff --git a/login/utmp_file.c b/login/utmp_file.c
8a8cfb
index da1baa6948d0eb39..812de8fd3d099ce9 100644
8a8cfb
--- a/login/utmp_file.c
8a8cfb
+++ b/login/utmp_file.c
8a8cfb
@@ -52,58 +52,71 @@ static struct utmp last_entry;
8a8cfb
 /* Do-nothing handler for locking timeout.  */
8a8cfb
 static void timeout_handler (int signum) {};
8a8cfb
 
8a8cfb
-/* LOCK_FILE(fd, type) failure_statement
8a8cfb
-     attempts to get a lock on the utmp file referenced by FD.  If it fails,
8a8cfb
-     the failure_statement is executed, otherwise it is skipped.
8a8cfb
-   LOCKING_FAILED()
8a8cfb
-     jumps into the UNLOCK_FILE macro and ensures cleanup of LOCK_FILE.
8a8cfb
-   UNLOCK_FILE(fd)
8a8cfb
-     unlocks the utmp file referenced by FD and performs the cleanup of
8a8cfb
-     LOCK_FILE.
8a8cfb
- */
8a8cfb
-#define LOCK_FILE(fd, type) \
8a8cfb
-{									      \
8a8cfb
-  struct flock fl;							      \
8a8cfb
-  struct sigaction action, old_action;					      \
8a8cfb
-  unsigned int old_timeout;						      \
8a8cfb
-									      \
8a8cfb
-  /* Cancel any existing alarm.  */					      \
8a8cfb
-  old_timeout = alarm (0);						      \
8a8cfb
-									      \
8a8cfb
-  /* Establish signal handler.  */					      \
8a8cfb
-  action.sa_handler = timeout_handler;					      \
8a8cfb
-  __sigemptyset (&action.sa_mask);					      \
8a8cfb
-  action.sa_flags = 0;							      \
8a8cfb
-  __sigaction (SIGALRM, &action, &old_action);				      \
8a8cfb
-									      \
8a8cfb
-  alarm (TIMEOUT);							      \
8a8cfb
-									      \
8a8cfb
-  /* Try to get the lock.  */						      \
8a8cfb
-  memset (&fl, '\0', sizeof (struct flock));				      \
8a8cfb
-  fl.l_type = (type);							      \
8a8cfb
-  fl.l_whence = SEEK_SET;						      \
8a8cfb
-  if (__fcntl64_nocancel ((fd), F_SETLKW, &fl) < 0)
8a8cfb
-
8a8cfb
-#define LOCKING_FAILED() \
8a8cfb
-  goto unalarm_return
8a8cfb
-
8a8cfb
-#define UNLOCK_FILE(fd) \
8a8cfb
-  /* Unlock the file.  */						      \
8a8cfb
-  fl.l_type = F_UNLCK;							      \
8a8cfb
-  __fcntl64_nocancel ((fd), F_SETLKW, &fl);				      \
8a8cfb
-									      \
8a8cfb
- unalarm_return:							      \
8a8cfb
-  /* Reset the signal handler and alarm.  We must reset the alarm	      \
8a8cfb
-     before resetting the handler so our alarm does not generate a	      \
8a8cfb
-     spurious SIGALRM seen by the user.  However, we cannot just set	      \
8a8cfb
-     the user's old alarm before restoring the handler, because then	      \
8a8cfb
-     it's possible our handler could catch the user alarm's SIGARLM	      \
8a8cfb
-     and then the user would never see the signal he expected.  */	      \
8a8cfb
-  alarm (0);								      \
8a8cfb
-  __sigaction (SIGALRM, &old_action, NULL);				      \
8a8cfb
-  if (old_timeout != 0)							      \
8a8cfb
-    alarm (old_timeout);						      \
8a8cfb
-} while (0)
8a8cfb
+
8a8cfb
+/* try_file_lock (LOCKING, FD, TYPE) returns true if the locking
8a8cfb
+   operation failed and recovery needs to be performed.
8a8cfb
+   (file_lock_restore (LOCKING) still needs to be called.)
8a8cfb
+
8a8cfb
+   file_unlock (FD) removes the lock (which must have been
8a8cfb
+   acquired).
8a8cfb
+
8a8cfb
+   file_lock_restore (LOCKING) is needed to clean up in both
8a8cfb
+   cases.  */
8a8cfb
+
8a8cfb
+struct file_locking
8a8cfb
+{
8a8cfb
+  struct sigaction old_action;
8a8cfb
+  unsigned int old_timeout;
8a8cfb
+};
8a8cfb
+
8a8cfb
+static bool
8a8cfb
+try_file_lock (struct file_locking *locking, int fd, int type)
8a8cfb
+{
8a8cfb
+  /* Cancel any existing alarm.  */
8a8cfb
+  locking->old_timeout = alarm (0);
8a8cfb
+
8a8cfb
+  /* Establish signal handler.  */
8a8cfb
+  struct sigaction action;
8a8cfb
+  action.sa_handler = timeout_handler;
8a8cfb
+  __sigemptyset (&action.sa_mask);
8a8cfb
+  action.sa_flags = 0;
8a8cfb
+  __sigaction (SIGALRM, &action, &locking->old_action);
8a8cfb
+
8a8cfb
+  alarm (TIMEOUT);
8a8cfb
+
8a8cfb
+  /* Try to get the lock.  */
8a8cfb
+ struct flock fl =
8a8cfb
+   {
8a8cfb
+    .l_type = type,
8a8cfb
+    fl.l_whence = SEEK_SET,
8a8cfb
+   };
8a8cfb
+ return __fcntl64_nocancel (fd, F_SETLKW, &fl) < 0;
8a8cfb
+}
8a8cfb
+
8a8cfb
+static void
8a8cfb
+file_unlock (int fd)
8a8cfb
+{
8a8cfb
+  struct flock fl =
8a8cfb
+    {
8a8cfb
+      .l_type = F_UNLCK,
8a8cfb
+    };
8a8cfb
+  __fcntl64_nocancel (fd, F_SETLKW, &fl);
8a8cfb
+}
8a8cfb
+
8a8cfb
+static void
8a8cfb
+file_lock_restore (struct file_locking *locking)
8a8cfb
+{
8a8cfb
+  /* Reset the signal handler and alarm.  We must reset the alarm
8a8cfb
+     before resetting the handler so our alarm does not generate a
8a8cfb
+     spurious SIGALRM seen by the user.  However, we cannot just set
8a8cfb
+     the user's old alarm before restoring the handler, because then
8a8cfb
+     it's possible our handler could catch the user alarm's SIGARLM
8a8cfb
+     and then the user would never see the signal he expected.  */
8a8cfb
+  alarm (0);
8a8cfb
+  __sigaction (SIGALRM, &locking->old_action, NULL);
8a8cfb
+  if (locking->old_timeout != 0)
8a8cfb
+    alarm (locking->old_timeout);
8a8cfb
+}
8a8cfb
 
8a8cfb
 #ifndef TRANSFORM_UTMP_FILE_NAME
8a8cfb
 # define TRANSFORM_UTMP_FILE_NAME(file_name) (file_name)
8a8cfb
@@ -153,16 +166,16 @@ __libc_getutent_r (struct utmp *buffer, struct utmp **result)
8a8cfb
       return -1;
8a8cfb
     }
8a8cfb
 
8a8cfb
-  LOCK_FILE (file_fd, F_RDLCK)
8a8cfb
+  struct file_locking fl;
8a8cfb
+  if (try_file_lock (&fl, file_fd, F_RDLCK))
8a8cfb
+    nbytes = 0;
8a8cfb
+  else
8a8cfb
     {
8a8cfb
-      nbytes = 0;
8a8cfb
-      LOCKING_FAILED ();
8a8cfb
+      /* Read the next entry.  */
8a8cfb
+      nbytes = __read_nocancel (file_fd, &last_entry, sizeof (struct utmp));
8a8cfb
+      file_unlock (file_fd);
8a8cfb
     }
8a8cfb
-
8a8cfb
-  /* Read the next entry.  */
8a8cfb
-  nbytes = __read_nocancel (file_fd, &last_entry, sizeof (struct utmp));
8a8cfb
-
8a8cfb
-  UNLOCK_FILE (file_fd);
8a8cfb
+  file_lock_restore (&fl);
8a8cfb
 
8a8cfb
   if (nbytes != sizeof (struct utmp))
8a8cfb
     {
8a8cfb
@@ -188,10 +201,12 @@ internal_getut_r (const struct utmp *id, struct utmp *buffer,
8a8cfb
 {
8a8cfb
   int result = -1;
8a8cfb
 
8a8cfb
-  LOCK_FILE (file_fd, F_RDLCK)
8a8cfb
+  struct file_locking fl;
8a8cfb
+  if (try_file_lock (&fl, file_fd, F_RDLCK))
8a8cfb
     {
8a8cfb
       *lock_failed = true;
8a8cfb
-      LOCKING_FAILED ();
8a8cfb
+      file_lock_restore (&fl);
8a8cfb
+      return -1;
8a8cfb
     }
8a8cfb
 
8a8cfb
   if (id->ut_type == RUN_LVL || id->ut_type == BOOT_TIME
8a8cfb
@@ -241,7 +256,8 @@ internal_getut_r (const struct utmp *id, struct utmp *buffer,
8a8cfb
   result = 0;
8a8cfb
 
8a8cfb
 unlock_return:
8a8cfb
-  UNLOCK_FILE (file_fd);
8a8cfb
+  file_unlock (file_fd);
8a8cfb
+  file_lock_restore (&fl);
8a8cfb
 
8a8cfb
   return result;
8a8cfb
 }
8a8cfb
@@ -287,10 +303,12 @@ __libc_getutline_r (const struct utmp *line, struct utmp *buffer,
8a8cfb
       return -1;
8a8cfb
     }
8a8cfb
 
8a8cfb
-  LOCK_FILE (file_fd, F_RDLCK)
8a8cfb
+  struct file_locking fl;
8a8cfb
+  if (try_file_lock (&fl, file_fd, F_RDLCK))
8a8cfb
     {
8a8cfb
       *result = NULL;
8a8cfb
-      LOCKING_FAILED ();
8a8cfb
+      file_lock_restore (&fl);
8a8cfb
+      return -1;
8a8cfb
     }
8a8cfb
 
8a8cfb
   while (1)
8a8cfb
@@ -318,7 +336,8 @@ __libc_getutline_r (const struct utmp *line, struct utmp *buffer,
8a8cfb
   *result = buffer;
8a8cfb
 
8a8cfb
 unlock_return:
8a8cfb
-  UNLOCK_FILE (file_fd);
8a8cfb
+  file_unlock (file_fd);
8a8cfb
+  file_lock_restore (&fl);
8a8cfb
 
8a8cfb
   return ((*result == NULL) ? -1 : 0);
8a8cfb
 }
8a8cfb
@@ -375,10 +394,11 @@ __libc_pututline (const struct utmp *data)
8a8cfb
 	}
8a8cfb
     }
8a8cfb
 
8a8cfb
-  LOCK_FILE (file_fd, F_WRLCK)
8a8cfb
+  struct file_locking fl;
8a8cfb
+  if (try_file_lock (&fl, file_fd, F_WRLCK))
8a8cfb
     {
8a8cfb
-      pbuf = NULL;
8a8cfb
-      LOCKING_FAILED ();
8a8cfb
+      file_lock_restore (&fl);
8a8cfb
+      return NULL;
8a8cfb
     }
8a8cfb
 
8a8cfb
   if (found < 0)
8a8cfb
@@ -421,7 +441,8 @@ __libc_pututline (const struct utmp *data)
8a8cfb
     }
8a8cfb
 
8a8cfb
  unlock_return:
8a8cfb
-  UNLOCK_FILE (file_fd);
8a8cfb
+  file_unlock (file_fd);
8a8cfb
+  file_lock_restore (&fl);
8a8cfb
 
8a8cfb
   return pbuf;
8a8cfb
 }
8a8cfb
@@ -450,8 +471,13 @@ __libc_updwtmp (const char *file, const struct utmp *utmp)
8a8cfb
   if (fd < 0)
8a8cfb
     return -1;
8a8cfb
 
8a8cfb
-  LOCK_FILE (fd, F_WRLCK)
8a8cfb
-    LOCKING_FAILED ();
8a8cfb
+  struct file_locking fl;
8a8cfb
+  if (try_file_lock (&fl, fd, F_WRLCK))
8a8cfb
+    {
8a8cfb
+      file_lock_restore (&fl);
8a8cfb
+      __close_nocancel_nostatus (fd);
8a8cfb
+      return -1;
8a8cfb
+    }
8a8cfb
 
8a8cfb
   /* Remember original size of log file.  */
8a8cfb
   offset = __lseek64 (fd, 0, SEEK_END);
8a8cfb
@@ -477,7 +503,8 @@ __libc_updwtmp (const char *file, const struct utmp *utmp)
8a8cfb
   result = 0;
8a8cfb
 
8a8cfb
 unlock_return:
8a8cfb
-  UNLOCK_FILE (fd);
8a8cfb
+  file_unlock (file_fd);
8a8cfb
+  file_lock_restore (&fl);
8a8cfb
 
8a8cfb
   /* Close WTMP file.  */
8a8cfb
   __close_nocancel_nostatus (fd);