Hello,

Two suggestions for 'who':

first,
the --help text for '-u/--users' option is not clear (IMHO).

'who --help' says:
   -u, --users       list users logged in

Whereas the Coreutils manual says:
"-u => After the login time, print the number of hours and minutes that the user has 
been idle. [...]"

And posix says ( 
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/who.html ):
"-u => Write "idle time" for each displayed user in addition to any other 
information. [...]".

so, except in the --help text, it talks about showing idle time.
Perhaps consider slightly modifying the usage text? (that's the first patch 
attached).


second,
would you consider adding a 'sort' option to sort output by different fields?
The attached patch implements this.
examples:
  who -a --sort=idle
  who -a --sort=name
  who -a --sort=line

Normally, piping to 'sort' should be enough, but the fields output by 'who' are 
not fixed, so sorting with whitespace delimiters is tricky,
and sorting by exact character position is messy, and depends on the option 
used.
Also, for sorting by idle time, the field conains mixed digits and characters (e.g. "." 
for recently active, and "old" for not active) - not easily sorted.

comments are welcomed,
 - Assaf
>From 3bbd0f213f285cb92082a76b75b873b49fda85a1 Mon Sep 17 00:00:00 2001
From: "A. Gordon" <[email protected]>
Date: Fri, 06 Feb 2015 20:45:49 -0500
Subject: [PATCH 0/4] *** SUBJECT HERE ***

*** BLURB HERE ***

A. Gordon (3):
  who: update usage text of '-u'
  who: extract stat(3) call to a function
  who: new option '--sort=WORD'

Assaf Gordon (1):
  who: sort with 'version-sort'

 bootstrap.conf |   1 +
 src/who.c      | 180 ++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 165 insertions(+), 16 deletions(-)

-- 
1.9.1

>From 48d2bc269f946a943dfeeb08297bdeedabe97ffe Mon Sep 17 00:00:00 2001
From: "A. Gordon" <[email protected]>
Date: Fri, 06 Feb 2015 20:45:49 -0500
Subject: [PATCH 1/4] who: update usage text of '-u'

* src/who.c: usage(): mention 'idle' in '-u' usage description.
---
 src/who.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/who.c b/src/who.c
index cc42d5f..079cf4e 100644
--- a/src/who.c
+++ b/src/who.c
@@ -664,7 +664,7 @@ Print information about users who are currently logged in.\n\
 "), stdout);
       fputs (_("\
   -T, -w, --mesg    add user's message status as +, - or ?\n\
-  -u, --users       list users logged in\n\
+  -u, --users       list users logged in with idle time\n\
       --message     same as -T\n\
       --writable    same as -T\n\
 "), stdout);
-- 
1.9.1


>From e22b87db6187fefaa1245877a03deaa4d4055662 Mon Sep 17 00:00:00 2001
From: "A. Gordon" <[email protected]>
Date: Fri, 06 Feb 2015 20:45:49 -0500
Subject: [PATCH 2/4] who: extract stat(3) call to a function

* src/who.c: print_user(): extract code which determines utmp's device
and calls stat(3) on it into a separate function.
utmp_stat(): takes a STRUCT_UTMP and calls stat(3) on the appropriate
device.
---
 src/who.c | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/src/who.c b/src/who.c
index 079cf4e..af0287b 100644
--- a/src/who.c
+++ b/src/who.c
@@ -324,20 +324,12 @@ is_tty_writable (struct stat const *pstat)
   return pstat->st_mode & S_IWGRP;
 }
 
-/* Send properly parsed USER_PROCESS info to print_line.  The most
-   recent boot time is BOOTTIME. */
-static void
-print_user (const STRUCT_UTMP *utmp_ent, time_t boottime)
+/* calls stat(3) on the utmp's line device.
+   returns zero if stat(3) succeeded. results stored in 'stats'.
+   returns non-zero otherwise. */
+static int
+utmp_stat (const STRUCT_UTMP *utmp_ent, struct stat /*OUT*/ *stats)
 {
-  struct stat stats;
-  time_t last_change;
-  char mesg;
-  char idlestr[IDLESTR_LEN + 1];
-  static char *hoststr;
-#if HAVE_UT_HOST
-  static size_t hostlen;
-#endif
-
 #define DEV_DIR_WITH_TRAILING_SLASH "/dev/"
 #define DEV_DIR_LEN (sizeof (DEV_DIR_WITH_TRAILING_SLASH) - 1)
 
@@ -352,7 +344,25 @@ print_user (const STRUCT_UTMP *utmp_ent, time_t boottime)
     p = stpcpy (p, DEV_DIR_WITH_TRAILING_SLASH);
   stzncpy (p, utmp_ent->ut_line, sizeof (utmp_ent->ut_line));
 
-  if (stat (line, &stats) == 0)
+  return stat (line, stats);
+}
+
+/* Send properly parsed USER_PROCESS info to print_line.  The most
+   recent boot time is BOOTTIME. */
+static void
+print_user (const STRUCT_UTMP *utmp_ent, time_t boottime)
+{
+  struct stat stats;
+  time_t last_change;
+  char mesg;
+  char idlestr[IDLESTR_LEN + 1];
+  static char *hoststr;
+#if HAVE_UT_HOST
+  static size_t hostlen;
+#endif
+  PIDSTR_DECL_AND_INIT (pidstr, utmp_ent);
+
+  if (utmp_stat (utmp_ent, &stats) == 0)
     {
       mesg = is_tty_writable (&stats) ? '+' : '-';
       last_change = stats.st_atime;
-- 
1.9.1


>From 88c011e2369aa0d59ab93bae9af60ebfad716014 Mon Sep 17 00:00:00 2001
From: "A. Gordon" <[email protected]>
Date: Fri, 06 Feb 2015 20:45:49 -0500
Subject: [PATCH 3/4] who: new option '--sort=WORD'

* src/who.c: support sorting output with '--sort=WORD'.
usage(): mention new option.
main(): handle new option.
who(): call sort_utmp().
sort_utmp(): sorts the STRUCT_UTMP array based on chosen sort option.
utmp_compare_login_time(), utmp_compare_idle_time(), utmp_compare_name()
utmp_compare_line(), utmp_compare_pid(): qsort-comptabile comparison
functions.
sort_by,sort_args,sort_types: new variables for sort options.
---
 src/who.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 138 insertions(+), 1 deletion(-)

diff --git a/src/who.c b/src/who.c
index af0287b..f75c527 100644
--- a/src/who.c
+++ b/src/who.c
@@ -32,6 +32,7 @@
 
 #include "c-ctype.h"
 #include "canon-host.h"
+#include "argmatch.h"
 #include "readutmp.h"
 #include "error.h"
 #include "hard-locale.h"
@@ -153,10 +154,34 @@ static bool my_line_only;
 static char const *time_format;
 static int time_format_width;
 
+/* Sort options */
+enum sort_type
+{
+  SORT_NONE = 0,
+  SORT_LOGIN_TIME,
+  SORT_IDLE_TIME,
+  SORT_LINE,
+  SORT_NAME,
+  SORT_PID
+};
+
+static char const *const sort_args[] =
+{
+  "none", "login", "idle", "line", "name", "pid", NULL
+};
+static char const sort_types[] =
+{
+  SORT_NONE, SORT_LOGIN_TIME, SORT_IDLE_TIME,
+  SORT_LINE, SORT_NAME, SORT_PID, 0
+};
+
+static enum sort_type sort_by = SORT_NONE;
+
 /* for long options with no corresponding short option, use enum */
 enum
 {
-  LOOKUP_OPTION = CHAR_MAX + 1
+  LOOKUP_OPTION = CHAR_MAX + 1,
+  SORT_OPTION
 };
 
 static struct option const longopts[] =
@@ -173,6 +198,7 @@ static struct option const longopts[] =
   {"process", no_argument, NULL, 'p'},
   {"runlevel", no_argument, NULL, 'r'},
   {"short", no_argument, NULL, 's'},
+  {"sort",  required_argument, NULL, SORT_OPTION},
   {"time", no_argument, NULL, 't'},
   {"users", no_argument, NULL, 'u'},
   {"writable", no_argument, NULL, 'T'},
@@ -621,6 +647,107 @@ scan_entries (size_t n, const STRUCT_UTMP *utmp_buf)
     }
 }
 
+
+/* qsort(3)-compatible comparision function,
+   comparing login time of utmp structures */
+static int
+utmp_compare_login_time (const void *a, const void *b)
+{
+  const STRUCT_UTMP *utmp_a = (STRUCT_UTMP*)a;
+  const STRUCT_UTMP *utmp_b = (STRUCT_UTMP*)b;
+
+  const time_t tm_a = UT_TIME_MEMBER (utmp_a);
+  const time_t tm_b = UT_TIME_MEMBER (utmp_b);
+
+  return tm_a-tm_b;
+}
+
+/* qsort(3)-compatible comparision function,
+   comparing idle time of utmp structures (if available,
+   based on the line device of each utmp entry).
+   entries without line devices are sorted last. */
+static int
+utmp_compare_idle_time (const void *a, const void *b)
+{
+  const STRUCT_UTMP *utmp_a = (STRUCT_UTMP*)a;
+  const STRUCT_UTMP *utmp_b = (STRUCT_UTMP*)b;
+
+  struct stat st_a, st_b;
+  time_t idle_a, idle_b;
+
+  idle_a = (utmp_stat (utmp_a, &st_a) == 0) ? st_a.st_atime : (time_t)-1;
+  idle_b = (utmp_stat (utmp_b, &st_b) == 0) ? st_b.st_atime : (time_t)-1;
+
+  return idle_b - idle_a;
+}
+
+/* qsort(3)-compatible comparision function,
+   comparing user names of utmp entries.
+   identical user names are sorted by login time. */
+static int
+utmp_compare_name (const void *a, const void *b)
+{
+  const STRUCT_UTMP *utmp_a = (STRUCT_UTMP*)a;
+  const STRUCT_UTMP *utmp_b = (STRUCT_UTMP*)b;
+
+  const int i = strcmp (UT_USER (utmp_a), UT_USER (utmp_b));
+  return ( (i!=0) ? i : utmp_compare_login_time (a, b));
+}
+
+/* qsort(3)-compatible comparision function,
+   comparing names of line devices of each utmp entry. */
+static int
+utmp_compare_line (const void *a, const void *b)
+{
+  const STRUCT_UTMP *utmp_a = (STRUCT_UTMP*)a;
+  const STRUCT_UTMP *utmp_b = (STRUCT_UTMP*)b;
+
+  return strcmp (utmp_a->ut_line, utmp_b->ut_line);
+}
+
+/* qsort(3)-compatible comparision function,
+   comparing PIDs associated with each utmp entry. */
+static int
+utmp_compare_pid (const void *a, const void *b)
+{
+  const STRUCT_UTMP *utmp_a = (STRUCT_UTMP*)a;
+  const STRUCT_UTMP *utmp_b = (STRUCT_UTMP*)b;
+
+  return UT_PID (utmp_a) - UT_PID (utmp_b);
+}
+
+/* sorts in-place the array of utmp entries, based on the user-specified
+   sorting option. */
+static void
+sort_utmp (STRUCT_UTMP *utmp_buf, size_t n_users)
+{
+  switch (sort_by)
+    {
+    case SORT_LOGIN_TIME:
+      qsort (utmp_buf, n_users, sizeof (*utmp_buf), utmp_compare_login_time);
+      break;
+
+    case SORT_IDLE_TIME:
+      qsort (utmp_buf, n_users, sizeof (*utmp_buf), utmp_compare_idle_time);
+      break;
+
+    case SORT_NAME:
+      qsort (utmp_buf, n_users, sizeof (*utmp_buf), utmp_compare_name);
+      break;
+
+    case SORT_LINE:
+      qsort (utmp_buf, n_users, sizeof (*utmp_buf), utmp_compare_line);
+      break;
+
+    case SORT_PID:
+      qsort (utmp_buf, n_users, sizeof (*utmp_buf), utmp_compare_pid);
+      break;
+
+    case SORT_NONE:
+      break;
+    }
+}
+
 /* Display a list of who is on the system, according to utmp file FILENAME.
    Use read_utmp OPTIONS to read the file.  */
 static void
@@ -632,6 +759,8 @@ who (const char *filename, int options)
   if (read_utmp (filename, &n_users, &utmp_buf, options) != 0)
     error (EXIT_FAILURE, errno, "%s", filename);
 
+  sort_utmp (utmp_buf, n_users);
+
   if (short_list)
     list_entries_who (n_users, utmp_buf);
   else
@@ -678,6 +807,10 @@ Print information about users who are currently logged in.\n\
       --message     same as -T\n\
       --writable    same as -T\n\
 "), stdout);
+      fputs (_("\
+      --sort=WORD   sort according to WORD:\n\
+                      none (default), login, idle, name, line, pid\n\
+"), stdout);
       fputs (HELP_OPTION_DESCRIPTION, stdout);
       fputs (VERSION_OPTION_DESCRIPTION, stdout);
       printf (_("\
@@ -788,6 +921,10 @@ main (int argc, char **argv)
           do_lookup = true;
           break;
 
+        case SORT_OPTION:
+          sort_by = XARGMATCH ("--sort", optarg, sort_args, sort_types);
+          break;
+
         case_GETOPT_HELP_CHAR;
 
         case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
-- 
1.9.1


>From 3bbd0f213f285cb92082a76b75b873b49fda85a1 Mon Sep 17 00:00:00 2001
From: Assaf Gordon <[email protected]>
Date: Fri, 06 Feb 2015 20:45:49 -0500
Subject: [PATCH 4/4] who: sort with 'version-sort'

* bootstrap.conf: include 'strverscmp'
* src/who.c: utmp_compare_line(): use 'strverscmp' instead of 'strcmp'.
---
 bootstrap.conf | 1 +
 src/who.c      | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index 4283140..5db4563 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -230,6 +230,7 @@ gnulib_modules="
   strtod
   strtoimax
   strtoumax
+  strverscmp
   symlink
   sys_ioctl
   sys_resource
diff --git a/src/who.c b/src/who.c
index f75c527..a3249f5 100644
--- a/src/who.c
+++ b/src/who.c
@@ -26,6 +26,7 @@
 #include <config.h>
 #include <getopt.h>
 #include <stdio.h>
+#include <string.h>
 
 #include <sys/types.h>
 #include "system.h"
@@ -702,7 +703,7 @@ utmp_compare_line (const void *a, const void *b)
   const STRUCT_UTMP *utmp_a = (STRUCT_UTMP*)a;
   const STRUCT_UTMP *utmp_b = (STRUCT_UTMP*)b;
 
-  return strcmp (utmp_a->ut_line, utmp_b->ut_line);
+  return strverscmp (utmp_a->ut_line, utmp_b->ut_line);
 }
 
 /* qsort(3)-compatible comparision function,
-- 
1.9.1

Reply via email to