Re: [PATCH] winsup/cygwin/libc/strptime.cc(__strptime) add strptime %s
On Aug 23 12:51, Brian Inglis wrote:
> 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;
Funny enough, in other places in Cygwin we call this temp variable
"save_errno" :)
Alternatively, since you're in C++ code, you can use the save_errno
class, like this:
{
save_errno save;
[do your thing]
}
The destructor of save_errno will restore errno.
Since the code as such is fine, it's your choice if you want to stick
to it or use one of the above. Just give the word.
Thanks,
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
signature.asc
Description: PGP signature
Re: winsup/cygwin/libc/strptime.cc(__strptime) strptime %F issue
On Aug 23 13:25, Brian Inglis wrote:
> 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.
Ok, I admit I didn't understand this fully. What is ''?
Can you give a real world example?
> So do you want compatible support or just the quick fix?
Quick and then right? Fixing this in two steps is just as well.
Thanks,
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
signature.asc
Description: PGP signature
Re: winsup/cygwin/libc/strptime.cc(__strptime) strptime %F issue
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]F, to support signed and
> > zero
> > filled fixed and variable length year fields in %F format.
>
> Ok, I admit I didn't understand this fully. What is ''?
> Can you give a real world example?
>
> > So do you want compatible support or just the quick fix?
>
> Quick and then right? Fixing this in two steps is just as well.
Btw., FreeBSD's _strptime only calls _strptime recursively, without any
checks for field width:
case 'F':
buf = _strptime(buf, "%Y-%m-%d", tm, GMTp, locale);
if (buf == NULL)
return (NULL);
flags |= FLAG_MONTH | FLAG_MDAY | FLAG_YEAR;
break;
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
signature.asc
Description: PGP signature
Re: [PATCH] winsup/cygwin/libc/strptime.cc(__strptime) add strptime %s
On 2017-08-24 03:25, Corinna Vinschen wrote:
> On Aug 23 12:51, Brian Inglis wrote:
>> 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;
>
> Funny enough, in other places in Cygwin we call this temp variable
> "save_errno" :)
>
>
> Alternatively, since you're in C++ code, you can use the save_errno
> class, like this:
>
> {
> save_errno save;
>
> [do your thing]
> }
>
> The destructor of save_errno will restore errno.
>
> Since the code as such is fine, it's your choice if you want to stick
> to it or use one of the above. Just give the word.
Useful - wish I'd known - stick to your standards and keep it clean - thanks.
--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
Re: winsup/cygwin/libc/strptime.cc(__strptime) strptime %F issue
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]F, to support signed and
>>> zero
>>> filled fixed and variable length year fields in %F format.
>>
>> Ok, I admit I didn't understand this fully. What is ''?
>> Can you give a real world example?
Width prefix for %F minus six for "-mm-dd" to get the width for %Y.
Look at POSIX strftime %F handling
http://pubs.opengroup.org/onlinepubs/9699919799/functions/strftime.html
or
man 3p strftime | less +/\ F\
for what strftime allows and strptime should handle for symmetry and
consistency.
>>> So do you want compatible support or just the quick fix?
>>
>> Quick and then right? Fixing this in two steps is just as well.
>
> Btw., FreeBSD's _strptime only calls _strptime recursively, without any
> checks for field width:
>
> case 'F':
> buf = _strptime(buf, "%Y-%m-%d", tm, GMTp, locale);
> if (buf == NULL)
> return (NULL);
> flags |= FLAG_MONTH | FLAG_MDAY | FLAG_YEAR;
> break;
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.
--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
Re: [PATCH] winsup/cygwin/libc/strptime.cc(__strptime) add strptime %s
On 2017-08-24 03:25, Corinna Vinschen wrote:
> On Aug 23 12:51, Brian Inglis wrote:
>> 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.
> Funny enough, in other places in Cygwin we call this temp variable
> "save_errno" :)
> Alternatively, since you're in C++ code, you can use the save_errno
> class, like this:
> {
> save_errno save;
>
> [do your thing]
> }
> The destructor of save_errno will restore errno.
> Since the code as such is fine, it's your choice if you want to stick
> to it or use one of the above. Just give the word.
Changed to use that.
--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada
From 16855e2e241673e5cb98368a696114e38f62a4dc Mon Sep 17 00:00:00 2001
From: Brian Inglis
Date: Thu, 24 Aug 2017 13:24:28 -0600
Subject: [PATCH] winsup/cygwin/libc/strptime.cc(__strptime) add %s support to
strptime
---
winsup/cygwin/libc/strptime.cc | 20
1 file changed, 20 insertions(+)
diff --git a/winsup/cygwin/libc/strptime.cc b/winsup/cygwin/libc/strptime.cc
index 62dca6e5e..7c6cc2024 100644
--- a/winsup/cygwin/libc/strptime.cc
+++ b/winsup/cygwin/libc/strptime.cc
@@ -573,6 +573,26 @@ 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;
+ char *end;
+ save_errno save;
+
+ LEGAL_ALT(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;
+ 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.1
