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