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

[Brian Inglis]

On 2017-08-29 01:35, Corinna Vinschen wrote:
> Hi Brian,
> 
> On Aug 25 11:47, Corinna Vinschen wrote:
>> On Aug 24 11:11, Brian Inglis wrote:
>>> On 2017-08-24 03:40, Corinna Vinschen wrote:
>>>> On Aug 24 11:32, Corinna Vinschen wrote:
>>>>> On Aug 23 13:25, Brian Inglis wrote:
>>>>>> 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]<w-6>F, to support signed and zero
>>>>>> filled fixed and variable length year fields in %F format.
>>>> Btw., FreeBSD's _strptime only calls _strptime recursively, without any
>>>> checks for field width:
>>> As did Cygwin, which just did a goto recurse, before it was changed to support
>>> explicit width. Your call and option to go back and ignore it, patch bug, or
>>> forward and support flags and width based on strftime documentation.
>>
>> Well, I guess it depends on how much time you're willing to invest here.
>> If you're inclined to fix this per POSIX, you're welcome, of course.
> 
> My vacation is approaching, so I'd like to get out the 2.9.0 release
> next week.  Naturally I wonder if and how you're proceeding on this
> issue.  Would it make sense, perhaps, if you just send the quick fix
> so we can get 2.9.0 out?
Attached - got diverted during strptime testing due to time functions gmtime,
localtime, mktime, strftime not properly handling struct tm->tm_year == INT_MAX
=> year == INT_MAX + 1900 so year needs to be at least long in Cygwin 64, also
affecting tzcalc_limits, and depending on what is required to properly handle
time_t in Cygwin 32.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
-------------- next part --------------
From 19a3c20c705a576fee0f0e71a31f0c3ac553e612 Mon Sep 17 00:00:00 2001
From: Brian Inglis <Brian.Inglis@SystematicSW.ab.ca>
Date: Tue, 29 Aug 2017 11:25:43 -0600
Subject: [PATCH] winsup/cygwin/libc/strptime.cc(__strptime) fix %F width

---
 winsup/cygwin/libc/strptime.cc | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/libc/strptime.cc b/winsup/cygwin/libc/strptime.cc
index 7c6cc2024..081aca385 100644
--- a/winsup/cygwin/libc/strptime.cc
+++ b/winsup/cygwin/libc/strptime.cc
@@ -413,13 +413,15 @@ literal:
 		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)
+			  /* width max chars converted, default 10, < 6 => 6 */
+			  if (tmp && (char *) bp +
+				(!width ? 10 : width < 6 ? 6 : width) < tmp)
 			    return NULL;
 			  bp = (const unsigned char *) tmp;
+			  ymd |= SET_YMD;
 			  continue;
 			}
 
-- 
2.14.1