Clean up lib/utmp.c and lib/logind.c

[alejandro-colomar]

Cc: @Karlson2k , @ikerexxe

---

Revisions:

v2

• Use ENOSYS instead of ENOTSUP [@ikerexxe ]

$ git range-diff shadow/master gh/logind_utmp logind_utmp 
 1:  bfb57d2b =  1:  bfb57d2b lib/logind.c: Fix style
 2:  738ee5d9 =  2:  738ee5d9 lib/logind.c: Remove dead initializations
 3:  00a50759 =  3:  00a50759 lib/: Move utmp.c and logind.c prototypes to new "session_management.h" header
 4:  d3ae3cf1 =  4:  d3ae3cf1 lib/logind.c: get_session_host(): free() earlier, to avoid a goto
 5:  98425add =  5:  98425add lib/logind.c: Wrap libsystemd function in our wrapper get_session()
 6:  7c58ee42 =  6:  7c58ee42 lib/logind.c: Wrap libsystemd function in our wrapper session_get_remote_host()
 7:  7f0fee3e =  7:  7f0fee3e lib/utmp.c: get_session_host(): Fix memory leak
 8:  99d5669b !  8:  c47b572f lib/, src/: get_session_host(): Return the host
    @@ lib/utmp.c: get_current_utmp(void)
      #else
     -  *out = NULL;
     -  ret = -2;
    -+  errno = ENOTSUP;
    ++  errno = ENOSYS;
      #endif
     +  host = NULL;
     +out:
 9:  528997fb =  9:  4fcf0a3d lib/string/strdup/: STRNDUP(): Add macro
10:  2361b195 = 10:  f6b6ec9d lib/utmp.c: get_session_host(): Don't exit from library code

v3

• Avoid a goto [@hallyn ]

$ git range-diff shadow/master gh/logind_utmp logind_utmp 
 1:  bfb57d2b =  1:  bfb57d2b lib/logind.c: Fix style
 2:  738ee5d9 =  2:  738ee5d9 lib/logind.c: Remove dead initializations
 3:  00a50759 =  3:  00a50759 lib/: Move utmp.c and logind.c prototypes to new "session_management.h" header
 4:  d3ae3cf1 =  4:  d3ae3cf1 lib/logind.c: get_session_host(): free() earlier, to avoid a goto
 5:  98425add =  5:  98425add lib/logind.c: Wrap libsystemd function in our wrapper get_session()
 6:  7c58ee42 =  6:  7c58ee42 lib/logind.c: Wrap libsystemd function in our wrapper session_get_remote_host()
 7:  7f0fee3e =  7:  7f0fee3e lib/utmp.c: get_session_host(): Fix memory leak
 -:  -------- >  8:  25bbd1ca lib/utmp.c: get_session_host(): De-duplicate code
 8:  c47b572f !  9:  b40036ec lib/, src/: get_session_host(): Return the host
    @@ lib/utmp.c: get_current_utmp(void)
     +get_session_host(void)
      {
     -  int           ret = 0;
    -+  char          *host;
    ++  char          *host = NULL;
        struct utmpx  *ut;
      
    +-  *out = NULL;
    +-
        ut = get_current_utmp();
      
      #if defined(HAVE_STRUCT_UTMPX_UT_HOST)
    -   if ((ut != NULL) && (ut->ut_host[0] != '\0')) {
    +-  if ((ut != NULL) && (ut->ut_host[0] != '\0')) {
     -          *out = XSTRNDUP(ut->ut_host);
     -  } else {
    --          *out = NULL;
     -          ret = -2;
    +-  }
    ++  if ((ut != NULL) && (ut->ut_host[0] != '\0'))
     +          host = XSTRNDUP(ut->ut_host);
    -+          goto out;
    -   }
      #else
    --  *out = NULL;
     -  ret = -2;
     +  errno = ENOSYS;
      #endif
    -+  host = NULL;
    -+out:
    ++
        free(ut);
      
     -  return ret;
 9:  4fcf0a3d = 10:  181c56d7 lib/string/strdup/: STRNDUP(): Add macro
10:  f6b6ec9d ! 11:  660b4a26 lib/utmp.c: get_session_host(): Don't exit from library code
    @@ lib/utmp.c
     @@ lib/utmp.c: get_session_host(void)
      
      #if defined(HAVE_STRUCT_UTMPX_UT_HOST)
    -   if ((ut != NULL) && (ut->ut_host[0] != '\0')) {
    +   if ((ut != NULL) && (ut->ut_host[0] != '\0'))
     -          host = XSTRNDUP(ut->ut_host);
     +          host = STRNDUP(ut->ut_host);
    -           goto out;
    -   }
      #else
    +   errno = ENOSYS;
    + #endif

v4

• #include "config.h" with quotes. [@Karlson2k ]

$ git range-diff shadow/master gh/logind_utmp logind_utmp 
 1:  bfb57d2b =  1:  bfb57d2b lib/logind.c: Fix style
 2:  738ee5d9 =  2:  738ee5d9 lib/logind.c: Remove dead initializations
 3:  00a50759 !  3:  2def5951 lib/: Move utmp.c and logind.c prototypes to new "session_management.h" header
    @@ lib/session_management.h (new)
     +#define SHADOW_INCLUDE_LIB_SESSION_MANAGEMENT_H_
     +
     +
    -+#include <config.h>
    ++#include "config.h"
     +
     +
     +/**
 4:  d3ae3cf1 =  4:  3687176c lib/logind.c: get_session_host(): free() earlier, to avoid a goto
 5:  98425add =  5:  2060d338 lib/logind.c: Wrap libsystemd function in our wrapper get_session()
 6:  7c58ee42 =  6:  0df3cd6e lib/logind.c: Wrap libsystemd function in our wrapper session_get_remote_host()
 7:  7f0fee3e =  7:  85e3c05e lib/utmp.c: get_session_host(): Fix memory leak
 8:  25bbd1ca =  8:  0bfad9c0 lib/utmp.c: get_session_host(): De-duplicate code
 9:  b40036ec !  9:  e3513b02 lib/, src/: get_session_host(): Return the host
    @@ lib/logind.c: ATTR_MALLOC(free) static char *get_session(void);
      ## lib/session_management.h ##
     @@
      
    - #include <config.h>
    + #include "config.h"
      
     +#include <stdlib.h>
      
10:  181c56d7 ! 10:  3ad35e90 lib/string/strdup/: STRNDUP(): Add macro
    @@ lib/string/strdup/strndup.c (new)
     +// SPDX-License-Identifier: BSD-3-Clause
     +
     +
    -+#include <config.h>
    ++#include "config.h"
     +
     +#include "string/strdup/strndup.h"
     
    @@ lib/string/strdup/strndup.h (new)
     +#define SHADOW_INCLUDE_LIB_STRING_STRDUP_STRNDUP_H_
     +
     +
    -+#include <config.h>
    ++#include "config.h"
     +
     +#include <string.h>
     +
11:  660b4a26 = 11:  92829912 lib/utmp.c: get_session_host(): Don't exit from library code

v5

• Rebase

$ git rd 
 1:  bfb57d2b !  1:  38b8516c lib/logind.c: Fix style
    @@ lib/logind.c
     - *
     - * SPDX-License-Identifier:  BSD-3-Clause
     - */
    -+// SPDX-FileCopyrightText:  2023, Iker Pedrosa <[email protected]>
    -+// SPDX-License-Identifier:  BSD-3-Clause
    ++// SPDX-FileCopyrightText: 2023, Iker Pedrosa <[email protected]>
    ++// SPDX-License-Identifier: BSD-3-Clause
     +
      
    - #include <config.h>
    + #include "config.h"
      
     -#ident "$Id$"
     +#include <systemd/sd-login.h>
    @@ lib/logind.c
      
     -#include <systemd/sd-login.h>
      
    --int get_session_host (char **out)
    +-int get_session_host (char **out, pid_t main_pid)
     +int
    -+get_session_host(char **out)
    ++get_session_host(char **out, pid_t main_pid)
      {
     -    char *host = NULL;
     -    char *session = NULL;
    @@ lib/logind.c
     +  char *session = NULL;
     +  int ret;
      
    --    ret = sd_pid_get_session (getpid(), &session);
    +-    ret = sd_pid_get_session(main_pid, &session);
     -    if (ret < 0) {
     -        return ret;
     -    }
    @@ lib/logind.c
     -    if (ret < 0) {
     -        goto done;
     -    }
    -+  ret = sd_pid_get_session(getpid(), &session);
    ++  ret = sd_pid_get_session(main_pid, &session);
     +  if (ret < 0)
     +          return ret;
     +  ret = sd_session_get_remote_host(session, &host);
 2:  738ee5d9 !  2:  c8e0f20f lib/logind.c: Remove dead initializations
    @@ Commit message
      ## lib/logind.c ##
     @@
      int
    - get_session_host(char **out)
    + get_session_host(char **out, pid_t main_pid)
      {
     -  char *host = NULL;
     -  char *session = NULL;
    @@ lib/logind.c
     +  char  *host;
     +  char  *session;
      
    -   ret = sd_pid_get_session(getpid(), &session);
    +   ret = sd_pid_get_session(main_pid, &session);
        if (ret < 0)
     @@ lib/logind.c: unsigned long
      active_sessions_count(const char *name, MAYBE_UNUSED unsigned long limit)
 3:  2def5951 !  3:  95211a21 lib/: Move utmp.c and logind.c prototypes to new "session_management.h" header
    @@ lib/limits.c
     
      ## lib/logind.c ##
     @@
    - // SPDX-FileCopyrightText:  2023, Iker Pedrosa <[email protected]>
    -+// SPDX-FileCopyrightText:  2025, Alejandro Colomar <[email protected]>
    - // SPDX-License-Identifier:  BSD-3-Clause
    + // SPDX-FileCopyrightText: 2023, Iker Pedrosa <[email protected]>
    ++// SPDX-FileCopyrightText: 2025, Alejandro Colomar <[email protected]>
    + // SPDX-License-Identifier: BSD-3-Clause
      
      
    - #include <config.h>
    + #include "config.h"
      
     +#include "session_management.h"
     +
    @@ lib/prototypes.h: extern int set_filesize_limit (int blocks);
     - * @brief Get host for the current session
     - *
     - * @param[out] out Host name
    +- * @param[in] main_pid the PID of the main process (the parent PID if
    +- *                     the process forked itself)
     - *
     - * @return 0 or a positive integer if the host was obtained properly,
     - *         another value on error.
     - */
    --extern int get_session_host (char **out);
    +-extern int get_session_host (char **out, pid_t main_pid);
     -#ifndef ENABLE_LOGIND
     -/**
     - * @brief Update or create an utmp entry in utmp, wtmp, utmpw, or wtmpx
    @@ lib/prototypes.h: extern int set_filesize_limit (int blocks);
     - * @param[in] user username
     - * @param[in] tty tty
     - * @param[in] host hostname
    +- * @param[in] main_pid the PID of the main process (the parent PID if
    +- *                     the process forked itself)
     - *
     - * @return 0 if utmp was updated properly,
     - *         1 on error.
     - */
     -extern int update_utmp (const char *user,
     -                        const char *tty,
    --                        const char *host);
    +-                        const char *host,
    +-                        pid_t main_pid);
     -/**
     - * @brief Update the cumulative failure log
     - *
     - * @param[in] failent_user username
     - * @param[in] tty tty
     - * @param[in] host hostname
    +- * @param[in] main_pid the PID of the main process (the parent PID if
    +- *                     the process forked itself)
     - *
     - */
     -extern void record_failure(const char *failent_user,
     -                           const char *tty,
    --                           const char *hostname);
    +-                           const char *hostname,
    +-                           pid_t main_pid);
     -#endif /* ENABLE_LOGIND */
     -
     -/**
    @@ lib/session_management.h (new)
     +
     +#include "config.h"
     +
    ++#include <sys/types.h>
    ++
     +
     +/**
     + * @brief Get host for the current session
     + *
     + * @param[out] out Host name
    ++ * @param[in] main_pid the PID of the main process (the parent PID if
    ++ *                     the process forked itself)
     + *
     + * @return 0 or a positive integer if the host was obtained properly,
     + *         another value on error.
     + */
    -+int get_session_host(char **out);
    ++int get_session_host(char **out, pid_t main_pid);
     +
     +
     +#if !defined(ENABLE_LOGIND)
    @@ lib/session_management.h (new)
     + * @param[in] user username
     + * @param[in] tty tty
     + * @param[in] host hostname
    ++ * @param[in] main_pid the PID of the main process (the parent PID if
    ++ *                     the process forked itself)
     + *
     + * @return 0 if utmp was updated properly,
     + *         1 on error.
     + */
    -+int update_utmp(const char *user, const char *tty, const char *host);
    ++int update_utmp(const char *user, const char *tty, const char *host,
    ++    pid_t main_pid);
     +/**
     + * @brief Update the cumulative failure log
     + *
     + * @param[in] failent_user username
     + * @param[in] tty tty
     + * @param[in] host hostname
    ++ * @param[in] main_pid the PID of the main process (the parent PID if
    ++ *                     the process forked itself)
     + *
     + */
     +void record_failure(const char *failent_user, const char *tty,
    -+    const char *hostname);
    ++    const char *hostname, pid_t main_pid);
     +#endif /* !ENABLE_LOGIND */
     +
     +
    @@ lib/session_management.h (new)
      ## lib/utmp.c ##
     @@
      
    - #include <config.h>
    + #include "config.h"
      
     +#include "session_management.h"
     +
 4:  3687176c !  4:  bb38cff0 lib/logind.c: get_session_host(): free() earlier, to avoid a goto
    @@ Commit message
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## lib/logind.c ##
    -@@ lib/logind.c: get_session_host(char **out)
    +@@ lib/logind.c: get_session_host(char **out, pid_t main_pid)
        if (ret < 0)
                return ret;
        ret = sd_session_get_remote_host(session, &host);
 5:  2060d338 !  5:  5d73f429 lib/logind.c: Wrap libsystemd function in our wrapper get_session()
    @@ lib/logind.c
     +#include <pwd.h>
     +#include <stddef.h>
     +#include <stdlib.h>
    ++#include <sys/types.h>
     +#include <unistd.h>
     +
      #include <systemd/sd-login.h>
    @@ lib/logind.c
      #include "prototypes.h"
      
      
    -+ATTR_MALLOC(free) static char *get_session(void);
    ++ATTR_MALLOC(free) static char *pid_get_session(pid_t pid);
     +
     +
      int
    - get_session_host(char **out)
    + get_session_host(char **out, pid_t main_pid)
      {
    -@@ lib/logind.c: get_session_host(char **out)
    +@@ lib/logind.c: get_session_host(char **out, pid_t main_pid)
        char  *host;
        char  *session;
      
    --  ret = sd_pid_get_session(getpid(), &session);
    +-  ret = sd_pid_get_session(main_pid, &session);
     -  if (ret < 0)
     -          return ret;
    -+  session = get_session();
    ++  session = pid_get_session(main_pid);
     +  if (session == NULL)
     +          return errno;
     +
        ret = sd_session_get_remote_host(session, &host);
        free(session);
        if (ret < 0)
    -@@ lib/logind.c: get_session_host(char **out)
    +@@ lib/logind.c: get_session_host(char **out, pid_t main_pid)
        return 0;
      }
      
    @@ lib/logind.c: active_sessions_count(const char *name, MAYBE_UNUSED unsigned long
     +
     +
     +static char *
    -+get_session(void)
    ++pid_get_session(pid_t pid)
     +{
     +  int   e;
     +  char  *session;
     +
    -+  e = sd_pid_get_session(getpid(), &session);
    ++  e = sd_pid_get_session(pid, &session);
     +  if (e < 0) {
     +          errno = -e;
     +          return NULL;
 6:  0df3cd6e !  6:  f4e39432 lib/logind.c: Wrap libsystemd function in our wrapper session_get_remote_host()
    @@ lib/logind.c
     @@
      
      
    - ATTR_MALLOC(free) static char *get_session(void);
    + ATTR_MALLOC(free) static char *pid_get_session(pid_t pid);
     +ATTR_MALLOC(free) static char *session_get_remote_host(char *session);
      
      
      int
    - get_session_host(char **out)
    + get_session_host(char **out, pid_t main_pid)
      {
     -  int   ret;
        char  *host;
        char  *session;
      
    -@@ lib/logind.c: get_session_host(char **out)
    +@@ lib/logind.c: get_session_host(char **out, pid_t main_pid)
        if (session == NULL)
                return errno;
      
    @@ lib/logind.c: get_session_host(char **out)
      
        *out = host;
        return 0;
    -@@ lib/logind.c: get_session(void)
    +@@ lib/logind.c: pid_get_session(pid_t pid)
      
        return session;
      }
 7:  85e3c05e !  7:  5708040f lib/utmp.c: get_session_host(): Fix memory leak
    @@ Commit message
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## lib/utmp.c ##
    -@@ lib/utmp.c: get_session_host(char **out)
    +@@ lib/utmp.c: get_session_host(char **out, pid_t main_pid)
      #if defined(HAVE_STRUCT_UTMPX_UT_HOST)
        if ((ut != NULL) && (ut->ut_host[0] != '\0')) {
                *out = XSTRNDUP(ut->ut_host);
    @@ lib/utmp.c: get_session_host(char **out)
        } else {
                *out = NULL;
                ret = -2;
    -@@ lib/utmp.c: get_session_host(char **out)
    +@@ lib/utmp.c: get_session_host(char **out, pid_t main_pid)
        *out = NULL;
        ret = -2;
      #endif
 8:  0bfad9c0 !  8:  4525e661 lib/utmp.c: get_session_host(): De-duplicate code
    @@ Commit message
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## lib/utmp.c ##
    -@@ lib/utmp.c: get_session_host(char **out)
    +@@ lib/utmp.c: get_session_host(char **out, pid_t main_pid)
        int           ret = 0;
        struct utmpx  *ut;
      
     +  *out = NULL;
     +
    -   ut = get_current_utmp();
    +   ut = get_current_utmp(main_pid);
      
      #if defined(HAVE_STRUCT_UTMPX_UT_HOST)
        if ((ut != NULL) && (ut->ut_host[0] != '\0')) {
 9:  e3513b02 !  9:  b581254e lib/, src/: get_session_host(): Return the host
    @@ Commit message
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## lib/logind.c ##
    -@@ lib/logind.c: ATTR_MALLOC(free) static char *get_session(void);
    +@@ lib/logind.c: ATTR_MALLOC(free) static char *pid_get_session(pid_t pid);
      ATTR_MALLOC(free) static char *session_get_remote_host(char *session);
      
      
     -int
    --get_session_host(char **out)
    +-get_session_host(char **out, pid_t main_pid)
     +char *
    -+get_session_host(void)
    ++get_session_host(pid_t main_pid)
      {
        char  *host;
        char  *session;
      
    -   session = get_session();
    +   session = pid_get_session(main_pid);
        if (session == NULL)
     -          return errno;
     +          return NULL;
    @@ lib/session_management.h
      #include "config.h"
      
     +#include <stdlib.h>
    + #include <sys/types.h>
    + 
    ++#include "attr.h"
      
     -/**
     - * @brief Get host for the current session
     - *
     - * @param[out] out Host name
    +- * @param[in] main_pid the PID of the main process (the parent PID if
    +- *                     the process forked itself)
     - *
     - * @return 0 or a positive integer if the host was obtained properly,
     - *         another value on error.
     - */
    --int get_session_host(char **out);
    -+#include "attr.h"
    -+
    +-int get_session_host(char **out, pid_t main_pid);
     +
     +ATTR_MALLOC(free)
    -+char *get_session_host(void);
    ++char *get_session_host(pid_t main_pid);
      
      
      #if !defined(ENABLE_LOGIND)
     
      ## lib/utmp.c ##
    -@@ lib/utmp.c: get_current_utmp(void)
    +@@ lib/utmp.c: get_current_utmp(pid_t main_pid)
      }
      
      
     -int
    --get_session_host(char **out)
    +-get_session_host(char **out, pid_t main_pid)
     +char *
    -+get_session_host(void)
    ++get_session_host(pid_t main_pid)
      {
     -  int           ret = 0;
     +  char          *host = NULL;
    @@ lib/utmp.c: get_current_utmp(void)
      
     -  *out = NULL;
     -
    -   ut = get_current_utmp();
    +   ut = get_current_utmp(main_pid);
      
      #if defined(HAVE_STRUCT_UTMPX_UT_HOST)
     -  if ((ut != NULL) && (ut->ut_host[0] != '\0')) {
    @@ lib/utmp.c: get_current_utmp(void)
     
      ## src/login.c ##
     @@ src/login.c: int main (int argc, char **argv)
    -           exit (1);       /* must be a terminal */
        }
      
    --  err = get_session_host(&host);
    -+  host = get_session_host();
    +   initial_pid = getpid();
    +-  err = get_session_host(&host, initial_pid);
    ++  host = get_session_host(initial_pid);
        /*
         * Be picky if run by normal users (possible if installed setuid
         * root), but not if run by root.
10:  3ad35e90 = 10:  6b2491e3 lib/string/strdup/: STRNDUP(): Add macro
11:  92829912 ! 11:  efff3048 lib/utmp.c: get_session_host(): Don't exit from library code
    @@ lib/utmp.c
      
      #define UTX_LINESIZE  countof(memberof(struct utmpx, ut_line))
      
    -@@ lib/utmp.c: get_session_host(void)
    +@@ lib/utmp.c: get_session_host(pid_t main_pid)
      
      #if defined(HAVE_STRUCT_UTMPX_UT_HOST)
        if ((ut != NULL) && (ut->ut_host[0] != '\0'))

v6

• Assume ut_host exists.

$ git range-diff shadow/master gh/logind_utmp logind_utmp 
 1:  38b8516c =  1:  38b8516c lib/logind.c: Fix style
 2:  c8e0f20f =  2:  c8e0f20f lib/logind.c: Remove dead initializations
 3:  95211a21 =  3:  95211a21 lib/: Move utmp.c and logind.c prototypes to new "session_management.h" header
 4:  bb38cff0 =  4:  bb38cff0 lib/logind.c: get_session_host(): free() earlier, to avoid a goto
 5:  5d73f429 =  5:  5d73f429 lib/logind.c: Wrap libsystemd function in our wrapper get_session()
 6:  f4e39432 =  6:  f4e39432 lib/logind.c: Wrap libsystemd function in our wrapper session_get_remote_host()
 7:  5708040f =  7:  5708040f lib/utmp.c: get_session_host(): Fix memory leak
 8:  4525e661 =  8:  4525e661 lib/utmp.c: get_session_host(): De-duplicate code
 9:  b581254e =  9:  b581254e lib/, src/: get_session_host(): Return the host
10:  6b2491e3 = 10:  6b2491e3 lib/string/strdup/: STRNDUP(): Add macro
11:  efff3048 = 11:  efff3048 lib/utmp.c: get_session_host(): Don't exit from library code
 -:  -------- > 12:  f22dfbd7 configure.ac, lib/utmp.c: Assume utmpx(5) has ut_host

v7

• Reorder commits, so that one can be dropped, and an initialization can be replaced by an assignment.

$ git range-diff shadow/master gh/logind_utmp logind_utmp 
 1:  38b8516c =  1:  38b8516c lib/logind.c: Fix style
 2:  c8e0f20f =  2:  c8e0f20f lib/logind.c: Remove dead initializations
 3:  95211a21 =  3:  95211a21 lib/: Move utmp.c and logind.c prototypes to new "session_management.h" header
 4:  bb38cff0 =  4:  bb38cff0 lib/logind.c: get_session_host(): free() earlier, to avoid a goto
 5:  5d73f429 =  5:  5d73f429 lib/logind.c: Wrap libsystemd function in our wrapper get_session()
 6:  f4e39432 =  6:  f4e39432 lib/logind.c: Wrap libsystemd function in our wrapper session_get_remote_host()
 7:  5708040f <  -:  -------- lib/utmp.c: get_session_host(): Fix memory leak
 8:  4525e661 <  -:  -------- lib/utmp.c: get_session_host(): De-duplicate code
12:  f22dfbd7 !  7:  ffa5ad10 configure.ac, lib/utmp.c: Assume utmpx(5) has ut_host
    @@ configure.ac: AC_SYS_LARGEFILE
                        struct utmpx.ut_addr_v6,
     
      ## lib/utmp.c ##
    -@@ lib/utmp.c: get_session_host(pid_t main_pid)
    +@@ lib/utmp.c: get_session_host(char **out, pid_t main_pid)
      
        ut = get_current_utmp(main_pid);
      
     -#if defined(HAVE_STRUCT_UTMPX_UT_HOST)
    -   if ((ut != NULL) && (ut->ut_host[0] != '\0'))
    -           host = STRNDUP(ut->ut_host);
    +   if ((ut != NULL) && (ut->ut_host[0] != '\0')) {
    +           *out = XSTRNDUP(ut->ut_host);
    +           free (ut);
    +@@ lib/utmp.c: get_session_host(char **out, pid_t main_pid)
    +           *out = NULL;
    +           ret = -2;
    +   }
     -#else
    --  errno = ENOSYS;
    +-  *out = NULL;
    +-  ret = -2;
     -#endif
      
    -   free(ut);
    - 
    +   return ret;
    + }
     @@ lib/utmp.c: prepare_utmp(const char *name, const char *line, const char *host,
      
        if (NULL != host && !streq(host, ""))
 -:  -------- >  8:  f78826d0 lib/utmp.c: get_session_host(): Fix memory leak
 9:  b581254e !  9:  806dff1b lib/, src/: get_session_host(): Return the host
    @@ lib/utmp.c: get_current_utmp(pid_t main_pid)
     +get_session_host(pid_t main_pid)
      {
     -  int           ret = 0;
    -+  char          *host = NULL;
    ++  char          *host;
        struct utmpx  *ut;
      
    --  *out = NULL;
    --
        ut = get_current_utmp(main_pid);
      
    - #if defined(HAVE_STRUCT_UTMPX_UT_HOST)
     -  if ((ut != NULL) && (ut->ut_host[0] != '\0')) {
     -          *out = XSTRNDUP(ut->ut_host);
     -  } else {
    +-          *out = NULL;
     -          ret = -2;
     -  }
     +  if ((ut != NULL) && (ut->ut_host[0] != '\0'))
     +          host = XSTRNDUP(ut->ut_host);
    - #else
    --  ret = -2;
    -+  errno = ENOSYS;
    - #endif
    -+
    ++  else
    ++          host = NULL;
    + 
        free(ut);
      
     -  return ret;
10:  6b2491e3 = 10:  ab38ff9d lib/string/strdup/: STRNDUP(): Add macro
11:  efff3048 ! 11:  5a2eb4e8 lib/utmp.c: get_session_host(): Don't exit from library code
    @@ lib/utmp.c
      #define UTX_LINESIZE  countof(memberof(struct utmpx, ut_line))
      
     @@ lib/utmp.c: get_session_host(pid_t main_pid)
    +   ut = get_current_utmp(main_pid);
      
    - #if defined(HAVE_STRUCT_UTMPX_UT_HOST)
        if ((ut != NULL) && (ut->ut_host[0] != '\0'))
     -          host = XSTRNDUP(ut->ut_host);
     +          host = STRNDUP(ut->ut_host);
    - #else
    -   errno = ENOSYS;
    - #endif
    +   else
    +           host = NULL;
    + 

$ git diff gh/logind_utmp 
diff --git c/lib/utmp.c w/lib/utmp.c
index 4cc7a394..52f9303c 100644
--- c/lib/utmp.c
+++ w/lib/utmp.c
@@ -189,13 +189,15 @@ get_current_utmp(pid_t main_pid)
 char *
 get_session_host(pid_t main_pid)
 {
-       char          *host = NULL;
+       char          *host;
        struct utmpx  *ut;
 
        ut = get_current_utmp(main_pid);
 
        if ((ut != NULL) && (ut->ut_host[0] != '\0'))
                host = STRNDUP(ut->ut_host);
+       else
+               host = NULL;
 
        free(ut);
 

v7b

• Rebase

$ git rd 
 1:  38b8516c =  1:  0b8acee3 lib/logind.c: Fix style
 2:  c8e0f20f =  2:  7a7842e5 lib/logind.c: Remove dead initializations
 3:  95211a21 =  3:  c9b6f10d lib/: Move utmp.c and logind.c prototypes to new "session_management.h" header
 4:  bb38cff0 =  4:  d0074ac2 lib/logind.c: get_session_host(): free() earlier, to avoid a goto
 5:  5d73f429 =  5:  2e182f2d lib/logind.c: Wrap libsystemd function in our wrapper get_session()
 6:  f4e39432 =  6:  4d6d25d2 lib/logind.c: Wrap libsystemd function in our wrapper session_get_remote_host()
 7:  ffa5ad10 =  7:  198abd73 configure.ac, lib/utmp.c: Assume utmpx(5) has ut_host
 8:  f78826d0 =  8:  a3667fdf lib/utmp.c: get_session_host(): Fix memory leak
 9:  806dff1b =  9:  2a51a80e lib/, src/: get_session_host(): Return the host
10:  ab38ff9d = 10:  586f4554 lib/string/strdup/: STRNDUP(): Add macro
11:  5a2eb4e8 = 11:  540e2dbe lib/utmp.c: get_session_host(): Don't exit from library code

v8

• Rebase. The fix for the memory leak has already been merged separately.

$ git rd 
 1:  0b8acee3 =  1:  3620b38e lib/logind.c: Fix style
 2:  7a7842e5 =  2:  ed3ae816 lib/logind.c: Remove dead initializations
 3:  c9b6f10d =  3:  b811b97c lib/: Move utmp.c and logind.c prototypes to new "session_management.h" header
 4:  d0074ac2 =  4:  7004410c lib/logind.c: get_session_host(): free() earlier, to avoid a goto
 5:  2e182f2d =  5:  2f59752b lib/logind.c: Wrap libsystemd function in our wrapper get_session()
 6:  4d6d25d2 =  6:  eda9907a lib/logind.c: Wrap libsystemd function in our wrapper session_get_remote_host()
 7:  198abd73 !  7:  29bc7157 configure.ac, lib/utmp.c: Assume utmpx(5) has ut_host
    @@ lib/utmp.c: get_session_host(char **out, pid_t main_pid)
     -#if defined(HAVE_STRUCT_UTMPX_UT_HOST)
        if ((ut != NULL) && (ut->ut_host[0] != '\0')) {
                *out = XSTRNDUP(ut->ut_host);
    -           free (ut);
    -@@ lib/utmp.c: get_session_host(char **out, pid_t main_pid)
    +   } else {
                *out = NULL;
                ret = -2;
        }
    @@ lib/utmp.c: get_session_host(char **out, pid_t main_pid)
     -  ret = -2;
     -#endif
      
    -   return ret;
    - }
    +   free(ut);
    + 
     @@ lib/utmp.c: prepare_utmp(const char *name, const char *line, const char *host,
      
        if (NULL != host && !streq(host, ""))
 8:  a3667fdf <  -:  -------- lib/utmp.c: get_session_host(): Fix memory leak
 9:  2a51a80e =  8:  7e7fcc3b lib/, src/: get_session_host(): Return the host
10:  586f4554 =  9:  89a53850 lib/string/strdup/: STRNDUP(): Add macro
11:  540e2dbe = 10:  e0bdc496 lib/utmp.c: get_session_host(): Don't exit from library code

v8b

• Rebase

$ git rd 
 1:  3620b38e =  1:  24a29ca2 lib/logind.c: Fix style
 2:  ed3ae816 =  2:  18cf9dd8 lib/logind.c: Remove dead initializations
 3:  b811b97c =  3:  2b41b114 lib/: Move utmp.c and logind.c prototypes to new "session_management.h" header
 4:  7004410c =  4:  795d0c23 lib/logind.c: get_session_host(): free() earlier, to avoid a goto
 5:  2f59752b =  5:  6dd5e258 lib/logind.c: Wrap libsystemd function in our wrapper get_session()
 6:  eda9907a =  6:  564522f7 lib/logind.c: Wrap libsystemd function in our wrapper session_get_remote_host()
 7:  29bc7157 =  7:  7662e3b6 configure.ac, lib/utmp.c: Assume utmpx(5) has ut_host
 8:  7e7fcc3b =  8:  b810ce7a lib/, src/: get_session_host(): Return the host
 9:  89a53850 =  9:  4d7b55c1 lib/string/strdup/: STRNDUP(): Add macro
10:  e0bdc496 = 10:  95781090 lib/utmp.c: get_session_host(): Don't exit from library code

v8c

• Rebase

$ git rd 
 1:  24a29ca2 =  1:  0d758316 lib/logind.c: Fix style
 2:  18cf9dd8 =  2:  0d68bd32 lib/logind.c: Remove dead initializations
 3:  2b41b114 !  3:  0c41d4bc lib/: Move utmp.c and logind.c prototypes to new "session_management.h" header
    @@ lib/Makefile.am: libshadow_la_SOURCES = \
     +  session_management.h \
        setugid.c \
        setupenv.c \
    -   sgetgrent.c \
    +   sgroupio.c \
     
      ## lib/limits.c ##
     @@
    @@ src/login.c
      #include "prototypes.h"
      #include "pwauth.h"
     +#include "session_management.h"
    + #include "shadow/gshadow/endsgent.h"
      #include "shadowlog.h"
      #include "string/memset/memzero.h"
    - #include "string/sprintf/snprintf.h"
     
      ## tests/unit/test_logind.c ##
     @@
 4:  795d0c23 =  4:  0d30f58e lib/logind.c: get_session_host(): free() earlier, to avoid a goto
 5:  6dd5e258 =  5:  cd722c36 lib/logind.c: Wrap libsystemd function in our wrapper get_session()
 6:  564522f7 =  6:  1373b72e lib/logind.c: Wrap libsystemd function in our wrapper session_get_remote_host()
 7:  7662e3b6 =  7:  9e384d21 configure.ac, lib/utmp.c: Assume utmpx(5) has ut_host
 8:  b810ce7a =  8:  ff162ed1 lib/, src/: get_session_host(): Return the host
 9:  4d7b55c1 =  9:  0a6f87fe lib/string/strdup/: STRNDUP(): Add macro
10:  95781090 = 10:  b0a25760 lib/utmp.c: get_session_host(): Don't exit from library code

[Karlson2k]

I'd move interfaces to special headers, like "utmp_functions.h" and "logind_functions.h" and name functions differently, to avoid potential conflict.
This would also give you a freedom to use different prototypes: for example logind_active_sessions_count() could have two parameters (without limit), while utmp_active_sessions_count() could have two parameters.

A third header "session_management.h" can include first two headers and route function calls to the real implementation.
For example:

#ifdef ENABLE_LOGIND
#define active_sessions_count(name, limit) logind_active_sessions_count(name)
#else
#define active_sessions_count(name, limit) utmp_active_sessions_count(name, limit)
#endif

This way you may skip some preprocessor macros in main code and get better optimisation even without LTO.

[alejandro-colomar]

I'd move interfaces to special headers, like "utmp_functions.h" and "logind_functions.h" and name functions differently, to avoid potential conflict. This would also give you a freedom to use different prototypes: for example logind_active_sessions_count() could have two parameters (without limit), while utmp_active_sessions_count() could have two parameters.

A third header "session_management.h" can include first two headers and route function calls to the real implementation. For example:

#ifdef ENABLE_LOGIND
#define active_sessions_count(name, limit) logind_active_sessions_count(name)
#else
#define active_sessions_count(name, limit) utmp_active_sessions_count(name, limit)
#endif

This way you may skip some preprocessor macros in main code and get better optimisation even without LTO.

I'd like to have a look at that after the fixes to the build system. I'm not familiar with autotools, and this kind of thing will require some conditionals there, I guess. Let's do this round of refactors first, and then we'll see what else we can improve.

[alejandro-colomar]

@ikerexxe , please have a look at this, so that it's easier for @Karlson2k to apply patches with a cleaner code.

[Karlson2k]

I'd like to have a look at that after the fixes to the build system. I'm not familiar with autotools, and this kind of thing will require some conditionals there, I guess. Let's do this round of refactors first, and then we'll see what else we can improve.

No problem. I can fix it easily later.

[alejandro-colomar]

All other checks for the presence of specific members,

Those are for members that might be supported by some systems, but which are not supported on Linux. We need those checks, as otherwise it wouldn't build on Linux. And if we remove that code, we might be breaking shadow on those systems, by not updating properly their utmp entries.

Our code will not stop building on systems if we remove code updating ut_syslen. It will just stop working correctly; but it will continue compiling there just fine.

[alejandro-colomar]

All other checks for the presence of specific members, specific functions (especially available only on other platforms, like explicit_bzero),

explicit_bzero(3) is available on glibc.

[Karlson2k]

Anyway, if the goal to support GNU/Linux only and you remove extra checks and use precisely what is (officially!) available on GNU/Linux, you will make code cleaner, simpler, easier to maintain and more secure. I'm talking about all the code, not only utmp interface part.

At the same time you achieve build failure on unsupported systems automatically.

That's my point.

Removing support to be built without POSIX-optional member, which break compatibility even with POSIX, but keeping all other fields and interfaces as optional, while they are always available on GNU/Linux and mandatory by POSIX, and thus extending support beyond both POSIX and GNU/Linux, makes a very little sense.

[Karlson2k]

To be more specific.

This change makes utmpx.ut_host mandatory.
In practice at least GNU/Linux, FreeBSD, NetBSD, OpenBSD have ut_host field. So support for all of them will not be changed by this patch.

The configure and the C code still support other optional fields, like utmpx.ut_name, which is available on NetBSD only. OpenBSD has ut_name in utmp (without "x') struct and has no utmpx struct so OpenBSD is completely unsupported. The field is not available on GNU/Linux.
However, NetBSD has other fields, like ut_ss, ut_exit. If for some reason shadow will be successfully compiled on NetBSD, it may successfully support ut_name, but other unsupported fields will be empty or, even worse, may contain garbage as the whole entry is not zeroed.

I'm suggesting to check in configure whether the target platform is GNU/Linux or GNU Hurd and abort if the platform is some other unsupported and untested platform.
Also, maybe it worth to zero-out entry completely before use to make sure that any unsupported fields are empty (or at least do not contain garbage).

[alejandro-colomar]

To be more specific.

This change makes utmpx.ut_host mandatory. In practice at least GNU/Linux, FreeBSD, NetBSD, OpenBSD have ut_host field. So support for all of them will not be changed by this patch.

The configure and the C code still support other optional fields, like utmpx.ut_name, which is available on NetBSD only. OpenBSD has ut_name in utmp (without "x') struct and has no utmpx struct so OpenBSD is completely unsupported. The field is not available on GNU/Linux. However, NetBSD has other fields, like ut_ss, ut_exit.

Is there any reason why glibc doesn't support those 2 fields that are present in NetBSD?

If for some reason shadow will be successfully compiled on NetBSD, it may successfully support ut_name, but other unsupported fields will be empty or, even worse, may contain garbage as the whole entry is not zeroed.

There are high chances that our code won't compile on NetBSD for other reasons. :)

I'm suggesting to check in configure whether the target platform is GNU/Linux or GNU Hurd and abort if the platform is some other unsupported and untested platform.

I'm hesitant of doing that. If anyone wants to use this in NetBSD and is ready to send bug reports, I'd be happy to fix anything they find. Just like we're doing now for Hurd.

Also, maybe it worth to zero-out entry completely before use to make sure that any unsupported fields are empty (or at least do not contain garbage).

Agree; we should clear all unused fields.

[alejandro-colomar]

Also, maybe it worth to zero-out entry completely before use to make sure that any unsupported fields are empty (or at least do not contain garbage).

Agree; we should clear all unused fields.

We're actually clearing them:

shadow/lib/utmp.c

Line 272 in 1f617f8

utent = XCALLOC(1, struct utmpx);

[Karlson2k]

To be more specific.
This change makes utmpx.ut_host mandatory. In practice at least GNU/Linux, FreeBSD, NetBSD, OpenBSD have ut_host field. So support for all of them will not be changed by this patch.
The configure and the C code still support other optional fields, like utmpx.ut_name, which is available on NetBSD only. OpenBSD has ut_name in utmp (without "x') struct and has no utmpx struct so OpenBSD is completely unsupported. The field is not available on GNU/Linux. However, NetBSD has other fields, like ut_ss, ut_exit.

Is there any reason why glibc doesn't support those 2 fields that are present in NetBSD?

Most probably the historical reasons.

If for some reason shadow will be successfully compiled on NetBSD, it may successfully support ut_name, but other unsupported fields will be empty or, even worse, may contain garbage as the whole entry is not zeroed.

There are high chances that our code won't compile on NetBSD for other reasons. :)

Yes, sure. Same for all other platforms. Why to keep compatibility with platforms that never tested and never used?
This makes code less strong.

I'm suggesting to check in configure whether the target platform is GNU/Linux or GNU Hurd and abort if the platform is some other unsupported and untested platform.

I'm hesitant of doing that. If anyone wants to use this in NetBSD and is ready to send bug reports, I'd be happy to fix anything they find. Just like we're doing now for Hurd.

GNU Hurd is limited to the same libc libraries as GNU/Linux.
The basic API is the same for apps.

Also, maybe it worth to zero-out entry completely before use to make sure that any unsupported fields are empty (or at least do not contain garbage).

Agree; we should clear all unused fields.

Just use memset() for the structure as unknown fields are... unknown. 😄

[alejandro-colomar]

Also, maybe it worth to zero-out entry completely before use to make sure that any unsupported fields are empty (or at least do not contain garbage).

Agree; we should clear all unused fields.

Just use memset() for the structure as unknown fields are... unknown. 😄

We're using calloc(3) already.