[PATCH] winsup/cygwin/libc/strptime.cc(__strptime) add strptime %s

2017-08-23 Thread Brian Inglis
On 2017-07-23 22:07, Brian Inglis wrote:
> On 2017-07-23 20:09, Lavrentiev, Anton (NIH/NLM/NCBI) [C] wrote:
>>> But that's just scanning a decimal integer to time_t.
>> It's not a question of whether I can or can't convert a string into an 
>> integer, rather it's a question about portability of code that uses %s for
>> both functions and expects it to work unchanged in the Cygwin environment.
>> Also, strptime() was designed to be a reversal to strftime() (from the
>> man-pages: the strptime() function is the converse function to strftime(3))
>> so both are supposed to "understand" the same basic set of formats. Because
>> of Cygwin's strptime() missing "%s", the following also does not work even
>> from command line:
>> $ date +"%s" | strptime "%s"
> Attached diff for proposed strptime %s and %F support.
> Let me know if you would prefer a different approach before I submit a git 
> format-patch.

Attached patch to support %s in Cygwin winsup libc strptime.cc __strptime().

This also enables support for %s in dateutils package strptime(1).

In case the issue comes up, if the user wants to support %s as in date(1) with a
preceding @ flag, they just have to include that verbatim before the format as
in "@%s".

Testing revealed a separate issue with %F format which I will follow up on in a
different thread.

Similar patch coming for newlib.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
From 11f950597e7f66132a2ce6c8120f7199ba02316f Mon Sep 17 00:00:00 2001
From: Brian Inglis 
Date: Tue, 22 Aug 2017 15:10:27 -0600
Subject: [PATCH] winsup/cygwin/libc/strptime.cc(__strptime) add strptime %s

---
 winsup/cygwin/libc/strptime.cc | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/winsup/cygwin/libc/strptime.cc b/winsup/cygwin/libc/strptime.cc
index 62dca6e5e..a7fef4985 100644
--- a/winsup/cygwin/libc/strptime.cc
+++ b/winsup/cygwin/libc/strptime.cc
@@ -573,6 +573,29 @@ literal:
bp = conv_num(bp, &tm->tm_sec, 0, 61, ALT_DIGITS);
continue;
 
+   case 's' :  /* The seconds since Unix epoch - GNU extension 
*/
+   {
+   long long sec;
+   time_t t;
+   int errno_save;
+   char *end;
+
+   LEGAL_ALT(0);
+   errno_save = errno;
+   errno = 0;
+   sec = strtoll_l ((char *)bp, &end, 10, locale);
+   t = sec;
+   if (end == (char *)bp
+   || errno != 0
+   || t != sec
+   || localtime_r (&t, tm) != tm)
+   return NULL;
+   errno = errno_save;
+   bp = (const unsigned char *)end;
+   ymd |= SET_YDAY | SET_WDAY | SET_YMD;
+   break;
+   }
+
case 'U':   /* The week of year, beginning on sunday. */
case 'W':   /* The week of year, beginning on monday. */
/*
-- 
2.14.0



Re: winsup/cygwin/libc/strptime.cc(__strptime) strptime %F issue

2017-08-23 Thread Brian Inglis
On 2017-08-23 12:51, Brian Inglis wrote:
> On 2017-07-23 22:07, Brian Inglis wrote:
>> On 2017-07-23 20:09, Lavrentiev, Anton (NIH/NLM/NCBI) [C] wrote:
 But that's just scanning a decimal integer to time_t.
>>> It's not a question of whether I can or can't convert a string into an 
>>> integer, rather it's a question about portability of code that uses %s
>>> for both functions and expects it to work unchanged in the Cygwin
>>> environment.
>>> Also, strptime() was designed to be a reversal to strftime() (from the 
>>> man-pages: the strptime() function is the converse function to
>>> strftime(3)) so both are supposed to "understand" the same basic set of
>>> formats. Because of Cygwin's strptime() missing "%s", the following also
>>> does not work even from command line:
>>> $ date +"%s" | strptime "%s"
> Testing revealed a separate issue with %F format which I will follow up on in
> a different thread.
Actually same thread, different subject.

Cygwin strptime(3) (also strptime(1)) fails with default width, without an
explicit width, because of the test in the following code:

case 'F':   /* The date as "%Y-%m-%d". */
{
  LEGAL_ALT(0);
  ymd |= SET_YMD;
  char *tmp = __strptime ((const char *) bp, "%Y-%m-%d",
  tm, era_info, alt_digits,
  locale);
  if (tmp && (uint) (tmp - (char *) bp) > width)
return NULL;
  bp = (const unsigned char *) tmp;
  continue;
}

as default width is zero so test fails and returns NULL.

Simple patch for this as with the other cases supporting width is to change the
test to:

  if (tmp && width && (uint) (tmp - (char *) bp) > width)

but this does not properly support [+0] flags or width in the format as
specified by glibc (latest POSIX punts on %F) for compatibility with strftime(),
affecting only the %Y format, supplying %[+0]F, to support signed and zero
filled fixed and variable length year fields in %F format.

So do you want compatible support or just the quick fix?

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada