PATCH: simplify printing of time stamps

[Urs Thuermann]

This patch simplifies considerably the functions do_time_format() and
format_date() used for %A, %C, and %T in the -printf predicate.
Instead of formatting the time using strftime(), then searching for
the seconds and appending a dot and the nanoseconds, one could simply
use nstrftime() from the GNU lib (already distributed with GNU
findutils) and use %N to get the nanoseconds.

The format strings used with nstrftime() are

        %F+%T.%N0       for %T+
        %s.%N0          for %T@
        %S.%N0          for %TS
        %T.%N0          for %TT

Only for %TX no nanoseconds are printed anymore, since we don't know
how %X will be formatted by nstrftime().

This patch also makes nanoseconds availabe for -printf by using %TN.

I would also suggest reverting the change in findutils-4.3.3 that %TT
and %TS print nanosecond resolution.  This change makes it difficult
to only print the seconds in the minute without nanoseconds.  With
this patch the user could then achieve the same effect using %TT.%TN
or %TS.%TN, respecitvely.

Therefore, I suggest deleting the line

        #define PRINT_NANOSECONDS

from my patch.  This would IMHO make GNU find cleaner, because people
knowing strftime(3) get what they expect from %TT and %TS, and it
makes GNU find consistent with the behavior of ls(1) and date(1) from
GNU coreutils.

urs


diff -rup findutils-4.4.2.orig/find/pred.c findutils-4.4.2/find/pred.c
--- findutils-4.4.2.orig/find/pred.c    2009-05-16 17:17:01.000000000 +0200
+++ findutils-4.4.2/find/pred.c 2012-12-26 11:42:41.529912314 +0100
@@ -33,6 +33,7 @@
 #include <locale.h>
 #include <openat.h>
 #include <ctype.h>
+#include "strftime.h"
 #include "xalloc.h"
 #include "dirname.h"
 #include "human.h"
@@ -2021,57 +2022,12 @@ launch (const struct buildcmd_control *c
 }
 
 
-static boolean
-scan_for_digit_differences(const char *p, const char *q,
-                          size_t *first, size_t *n)
-{
-  bool seen = false;
-  size_t i;
-
-  for (i=0; p[i] && q[i]; i++)
-    {
-      if (p[i] != q[i])
-       {
-         if (!isdigit((unsigned char)q[i]) || !isdigit ((unsigned char)q[i]))
-           return false;
-
-         if (!seen)
-           {
-             *first = i;
-             *n = 1;
-             seen = 1;
-           }
-         else
-           {
-             if (i-*first == *n)
-               {
-                 /* Still in the first sequence of differing digits. */
-                 ++*n;
-               }
-             else
-               {
-                 /* More than one differing contiguous character sequence. */
-                 return false;
-               }
-           }
-       }
-    }
-  if (p[i] || q[i])
-    {
-      /* strings are different lengths. */
-      return false;
-    }
-  return true;
-}
-
-
 static char*
-do_time_format (const char *fmt, const struct tm *p, const char *ns, size_t 
ns_size)
+do_time_format (const char *fmt, const struct tm *p, int ns)
 {
   static char *buf = NULL;
   static size_t buf_size;
   char *timefmt = NULL;
-  struct tm altered_time;
 
 
   /* If the format expands to nothing (%p in some locales, for
@@ -2083,15 +2039,6 @@ do_time_format (const char *fmt, const s
   timefmt = xmalloc (strlen(fmt) + 2u);
   sprintf (timefmt, "_%s", fmt);
 
-  /* altered_time is a similar time, but in which both
-   * digits of the seconds field are different.
-   */
-  altered_time = *p;
-  if (altered_time.tm_sec >= 11)
-    altered_time.tm_sec -= 11;
-  else
-    altered_time.tm_sec += 11;
-
   /* If we call strftime() with buf_size=0, the program will coredump
    * on Solaris, since it unconditionally writes the terminating null
    * character.
@@ -2104,46 +2051,10 @@ do_time_format (const char *fmt, const s
        * Therefore we do not check for (buf_used != 0) as the termination
        * condition.
        */
-      size_t buf_used = strftime (buf, buf_size, timefmt, p);
+           size_t buf_used = nstrftime (buf, buf_size, timefmt, p, 0, ns);
       if (buf_used             /* Conforming POSIX system */
          && (buf_used < buf_size)) /* Solaris workaround */
        {
-         char *altbuf;
-         size_t i = 0, n = 0;
-         size_t final_len = (buf_used
-                             + 1u /* for \0 */
-                             + ns_size);
-         buf = xrealloc (buf, final_len);
-         altbuf = xmalloc (final_len);
-         strftime (altbuf, buf_size, timefmt, &altered_time);
-
-         /* Find the seconds digits; they should be the only changed part.
-          * In theory the result of the two formatting operations could differ 
in
-          * more than just one sequence of decimal digits (for example %X might
-          * in theory return a spelled-out time like "thirty seconds past 
noon").
-          * When that happens, we just avoid inserting the nanoseconds field.
-          */
-         if (scan_for_digit_differences (buf, altbuf, &i, &n)
-             && (2==n) && !isdigit((unsigned char)buf[i+n]))
-           {
-             const size_t end_of_seconds = i + n;
-             const size_t suffix_len = buf_used-(end_of_seconds)+1;
-
-             /* Move the tail (including the \0).  Note that this
-              * is a move of an overlapping memory block, so we
-              * must use memmove instead of memcpy.  Then insert
-              * the nanoseconds (but not its trailing \0).
-              */
-             assert (end_of_seconds + ns_size + suffix_len == final_len);
-             memmove (buf+end_of_seconds+ns_size,
-                      buf+end_of_seconds,
-                      suffix_len);
-             memcpy (buf+i+n, ns, ns_size);
-           }
-         else
-           {
-             /* No seconds digits.  No need to insert anything. */
-           }
          /* The first character of buf is the underscore, which we actually
           * don't want.
           */
@@ -2169,90 +2080,33 @@ do_time_format (const char *fmt, const s
  * really runtime checks.  The assertions actually exist to verify
  * that the various buffers are correctly sized.
  */
+#define PRINT_NANOSECONDS
 static char *
 format_date (struct timespec ts, int kind)
 {
-  /* In theory, we use an extra 10 characters for 9 digits of
-   * nanoseconds and 1 for the decimal point.  However, the real
-   * world is more complex than that.
-   *
-   * For example, some systems return junk in the tv_nsec part of
-   * st_birthtime.  An example of this is the NetBSD-4.0-RELENG kernel
-   * (at Sat Mar 24 18:46:46 2007) running a NetBSD-3.1-RELEASE
-   * runtime and examining files on an msdos filesytem.  So for that
-   * reason we set NS_BUF_LEN to 32, which is simply "long enough" as
-   * opposed to "exactly the right size".  Note that the behaviour of
-   * NetBSD appears to be a result of the use of uninitialised data,
-   * as it's not 100% reproducible (more like 25%).
-   */
-  enum {
-    NS_BUF_LEN = 32,
-    DATE_LEN_PERCENT_APLUS=21  /* length of result of %A+ (it's longer than 
%c)*/
-  };
-  static char buf[128u+10u + MAX(DATE_LEN_PERCENT_APLUS,
-                           MAX (LONGEST_HUMAN_READABLE + 2, 
NS_BUF_LEN+64+200))];
-  char ns_buf[NS_BUF_LEN]; /* -.9999999990 (- sign can happen!)*/
-  int  charsprinted, need_ns_suffix;
   struct tm *tm;
-  char fmt[6];
-
-  /* human_readable() assumes we pass a buffer which is at least as
-   * long as LONGEST_HUMAN_READABLE.  We use an assertion here to
-   * ensure that no nasty unsigned overflow happend in our calculation
-   * of the size of buf.  Do the assertion here rather than in the
-   * code for %@ so that we find the problem quickly if it exists.  If
-   * you want to submit a patch to move this into the if statement, go
-   * ahead, I'll apply it.  But include performance timings
-   * demonstrating that the performance difference is actually
-   * measurable.
-   */
-  verify (sizeof(buf) >= LONGEST_HUMAN_READABLE);
-
-  charsprinted = 0;
-  need_ns_suffix = 0;
+  char fmt[10];
 
   /* Format the main part of the time. */
   if (kind == '+')
     {
-      strcpy (fmt, "%F+%T");
-      need_ns_suffix = 1;
+      strcpy (fmt, "%F+%T.%N0");
     }
-  else
+  else if (kind == '@')
     {
-      fmt[0] = '%';
-      fmt[1] = kind;
-      fmt[2] = '\0';
-
-      /* %a, %c, and %t are handled in ctime_format() */
-      switch (kind)
-       {
-       case 'S':
-       case 'T':
-       case 'X':
-       case '@':
-         need_ns_suffix = 1;
-         break;
-       default:
-         need_ns_suffix = 0;
-         break;
-       }
+      strcpy (fmt, "%s.%N0");
     }
-
-  if (need_ns_suffix)
+#ifdef PRINT_NANOSECONDS
+  else if (kind == 'S' || kind == 'T')
     {
-      /* Format the nanoseconds part.  Leave a trailing zero to
-       * discourage people from writing scripts which extract the
-       * fractional part of the timestamp by using column offsets.
-       * The reason for discouraging this is that in the future, the
-       * granularity may not be nanoseconds.
-       */
-      charsprinted = snprintf(ns_buf, NS_BUF_LEN, ".%09ld0", (long 
int)ts.tv_nsec);
-      assert (charsprinted < NS_BUF_LEN);
+      sprintf (fmt, "%%%c.%%N0", kind);
     }
+#endif
   else
     {
-      charsprinted = 0;
-      ns_buf[0] = 0;
+      fmt[0] = '%';
+      fmt[1] = kind;
+      fmt[2] = '\0';
     }
 
   if (kind != '@')
@@ -2260,17 +2114,50 @@ format_date (struct timespec ts, int kin
       tm = localtime (&ts.tv_sec);
       if (tm)
        {
-         char *s = do_time_format (fmt, tm, ns_buf, charsprinted);
+         char *s = do_time_format (fmt, tm, ts.tv_nsec);
          if (s)
            return s;
        }
     }
 
-  /* If we get to here, either the format was %@, or we have fallen back to it
-   * because strftime failed.
+  /* If we get to here, strftime has failed.
    */
   if (1)
     {
+      /* In theory, we use an extra 10 characters for 9 digits of
+       * nanoseconds and 1 for the decimal point.  However, the real
+       * world is more complex than that.
+       *
+       * For example, some systems return junk in the tv_nsec part of
+       * st_birthtime.  An example of this is the NetBSD-4.0-RELENG kernel
+       * (at Sat Mar 24 18:46:46 2007) running a NetBSD-3.1-RELEASE
+       * runtime and examining files on an msdos filesytem.  So for that
+       * reason we set NS_BUF_LEN to 32, which is simply "long enough" as
+       * opposed to "exactly the right size".  Note that the behaviour of
+       * NetBSD appears to be a result of the use of uninitialised data,
+       * as it's not 100% reproducible (more like 25%).
+       */
+      enum {
+       NS_BUF_LEN = 32,
+       DATE_LEN_PERCENT_APLUS=21    /* length of result of %A+ (it's longer 
than %c)*/
+      };
+      static char buf[128u+10u + MAX(DATE_LEN_PERCENT_APLUS,
+                                    MAX (LONGEST_HUMAN_READABLE + 2, 
NS_BUF_LEN+64+200))];
+      char ns_buf[NS_BUF_LEN]; /* -.9999999990 (- sign can happen!)*/
+      int  charsprinted;
+
+      /* human_readable() assumes we pass a buffer which is at least as
+       * long as LONGEST_HUMAN_READABLE.  We use an assertion here to
+       * ensure that no nasty unsigned overflow happend in our calculation
+       * of the size of buf.  Do the assertion here rather than in the
+       * code for %@ so that we find the problem quickly if it exists.  If
+       * you want to submit a patch to move this into the if statement, go
+       * ahead, I'll apply it.  But include performance timings
+       * demonstrating that the performance difference is actually
+       * measurable.
+       */
+      verify (sizeof(buf) >= LONGEST_HUMAN_READABLE);
+
       uintmax_t w = ts.tv_sec;
       size_t used, len, remaining;
 
@@ -2284,27 +2171,34 @@ format_date (struct timespec ts, int kin
       if (ts.tv_sec < 0)
        *--p = '-'; /* XXX: Ugh, relying on internal details of 
human_readable(). */
 
+      /* Format the nanoseconds part.  Leave a trailing zero to
+       * discourage people from writing scripts which extract the
+       * fractional part of the timestamp by using column offsets.
+       * The reason for discouraging this is that in the future, the
+       * granularity may not be nanoseconds.
+       */
+      charsprinted = snprintf(ns_buf, NS_BUF_LEN, ".%09ld0", (long 
int)ts.tv_nsec);
+      assert (charsprinted < NS_BUF_LEN);
+
       /* Add the nanoseconds part.  Because we cannot enforce a
        * particlar implementation of human_readable, we cannot assume
        * any particular value for (p-buf).  So we need to be careful
        * that there is enough space remaining in the buffer.
        */
-      if (need_ns_suffix)
-       {
-         len = strlen(p);
-         used = (p-buf) + len; /* Offset into buf of current end */
-         assert (sizeof buf > used); /* Ensure we can perform subtraction 
safely. */
-         remaining = sizeof buf - used - 1u; /* allow space for NUL */
-
-         if (strlen(ns_buf) >= remaining)
-           {
-             error(0, 0,
-                   "charsprinted=%ld but remaining=%lu: ns_buf=%s",
-                   (long)charsprinted, (unsigned long)remaining, ns_buf);
-           }
-         assert (strlen(ns_buf) < remaining);
-         strcat(p, ns_buf);
+      len = strlen(p);
+      used = (p-buf) + len;    /* Offset into buf of current end */
+      assert (sizeof buf > used); /* Ensure we can perform subtraction safely. 
*/
+      remaining = sizeof buf - used - 1u; /* allow space for NUL */
+
+      if (strlen(ns_buf) >= remaining)
+       {
+         error(0, 0,
+               "charsprinted=%ld but remaining=%lu: ns_buf=%s",
+               (long)charsprinted, (unsigned long)remaining, ns_buf);
        }
+      assert (strlen(ns_buf) < remaining);
+      strcat(p, ns_buf);
+
       return p;
     }
 }