public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] time: ensure failing strptime() tests are reported correctly
@ 2015-01-27 15:20 Vincent Bernat
  2015-01-27 16:02 ` [PATCH] time: in strptime(), make %z accept Z as a time zone Vincent Bernat
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Vincent Bernat @ 2015-01-27 15:20 UTC (permalink / raw)
  To: libc-alpha; +Cc: Vincent Bernat

From: Vincent Bernat <vincent@bernat.im>

---
 time/tst-strptime2.c | 5 +----
 time/tst-strptime3.c | 5 +----
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/time/tst-strptime2.c b/time/tst-strptime2.c
index bcd6cc8be98f..a08e6d7cb7de 100644
--- a/time/tst-strptime2.c
+++ b/time/tst-strptime2.c
@@ -52,10 +52,7 @@ do_test (void)
 	}
     }
 
-  if (result == 0)
-    puts ("all OK");
-
-  return 0;
+  return result;
 }
 
 #define TEST_FUNCTION do_test ()
diff --git a/time/tst-strptime3.c b/time/tst-strptime3.c
index 75b57c192871..d53f51ee2244 100644
--- a/time/tst-strptime3.c
+++ b/time/tst-strptime3.c
@@ -48,10 +48,7 @@ do_test (void)
       result = 1;
     }
 
-  if (result == 0)
-    puts ("all OK");
-
-  return 0;
+  return result;
 }
 
 #define TEST_FUNCTION do_test ()
-- 
2.1.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] time: in strptime(), make %z accept Z as a time zone
@ 2015-01-27 16:02 ` Vincent Bernat
  2015-03-06 10:11   ` Mike Frysinger
  0 siblings, 1 reply; 10+ messages in thread
From: Vincent Bernat @ 2015-01-27 16:02 UTC (permalink / raw)
  To: libc-alpha; +Cc: Vincent Bernat

From: Vincent Bernat <vincent@bernat.im>

In ISO 8601, the timezone can be 'Z' instead of using
digits. 2014-08-17T12:33:12+0000 is often expressed as
2014-08-17T12:33:12Z.
---
 ChangeLog            | 5 +++++
 time/strptime_l.c    | 9 +++++++--
 time/tst-strptime2.c | 1 +
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c6227b73c6c0..24c1a74c963d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2015-01-27  Vincent Bernat  <vincent@bernat.im>
+
+	[BZ #17886]
+	* time/strptime_l.c: Make %z accept Z as a valid time zone.
+
 2015-01-27  Andreas Krebbel  <Andreas.Krebbel@de.ibm.com>
 
 	* iconv/loop.c: Suppress array out of bound warning caused by GCC
diff --git a/time/strptime_l.c b/time/strptime_l.c
index 5640ccecced0..78882ec39aed 100644
--- a/time/strptime_l.c
+++ b/time/strptime_l.c
@@ -749,13 +749,18 @@ __strptime_internal (rp, fmt, tmp, statep LOCALE_PARAM)
 	    rp++;
 	  break;
 	case 'z':
-	  /* We recognize two formats: if two digits are given, these
+	  /* We recognize three formats: if two digits are given, these
 	     specify hours.  If fours digits are used, minutes are
-	     also specified.  */
+	     also specified. 'Z' is equivalent to +0000. */
 	  {
 	    val = 0;
 	    while (ISSPACE (*rp))
 	      ++rp;
+            if (*rp == 'Z')
+              {
+                rp++;
+                break;
+              }
 	    if (*rp != '+' && *rp != '-')
 	      return NULL;
 	    bool neg = *rp++ == '-';
diff --git a/time/tst-strptime2.c b/time/tst-strptime2.c
index a08e6d7cb7de..c22967a93026 100644
--- a/time/tst-strptime2.c
+++ b/time/tst-strptime2.c
@@ -17,6 +17,7 @@ static const struct
     { "1113472456 -1030", -37800 },
     { "1113472456 +0030", 1800 },
     { "1113472456 -0030", -1800 },
+    { "1113472456  Z", 0 },
     { "1113472456 -1330", LONG_MAX },
     { "1113472456 +1330", LONG_MAX },
     { "1113472456 -1060", LONG_MAX },
-- 
2.1.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] time: in strptime(), make %z accept [+-]HH:MM time zones
@ 2015-01-27 16:03 ` Vincent Bernat
  2015-03-06 10:21   ` Mike Frysinger
  0 siblings, 1 reply; 10+ messages in thread
From: Vincent Bernat @ 2015-01-27 16:03 UTC (permalink / raw)
  To: libc-alpha; +Cc: Vincent Bernat

From: Vincent Bernat <vincent@bernat.im>

In ISO 8601, +03:30 is a valid time zone. Currently, strptime() only
parses it as a 2-digit time zone an believes this is +03:00. This change
makes it accept a single semi-colon.

This fix BZ #17887.
---
 ChangeLog            |  3 +++
 time/strptime_l.c    | 18 ++++++++++++++----
 time/tst-strptime2.c |  9 +++++++++
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 24c1a74c963d..0f3e4bdc72a8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,9 @@
 	[BZ #17886]
 	* time/strptime_l.c: Make %z accept Z as a valid time zone.
 
+	[BZ #17887]
+	* time/strptime_l.c: Make %z accept [+-]HH:MM time zones.
+
 2015-01-27  Andreas Krebbel  <Andreas.Krebbel@de.ibm.com>
 
 	* iconv/loop.c: Suppress array out of bound warning caused by GCC
diff --git a/time/strptime_l.c b/time/strptime_l.c
index 78882ec39aed..3326e110d928 100644
--- a/time/strptime_l.c
+++ b/time/strptime_l.c
@@ -749,9 +749,11 @@ __strptime_internal (rp, fmt, tmp, statep LOCALE_PARAM)
 	    rp++;
 	  break;
 	case 'z':
-	  /* We recognize three formats: if two digits are given, these
-	     specify hours.  If fours digits are used, minutes are
-	     also specified. 'Z' is equivalent to +0000. */
+	  /* We recognize four formats: 1. if two digits are given,
+	     these specify hours.  2. If fours digits are used,
+	     minutes are also specified. 3. A semi-colon can be used
+	     to separate the two groups of two digits (HH:MM). 4. 'Z'
+	     is equivalent to +0000. */
 	  {
 	    val = 0;
 	    while (ISSPACE (*rp))
@@ -765,8 +767,16 @@ __strptime_internal (rp, fmt, tmp, statep LOCALE_PARAM)
 	      return NULL;
 	    bool neg = *rp++ == '-';
 	    int n = 0;
-	    while (n < 4 && *rp >= '0' && *rp <= '9')
+	    while (n < 4 &&
+                   ((*rp >= '0' && *rp <= '9') ||
+                    (*rp == ':' && n == 2)))
 	      {
+                if (*rp == ':')
+                  {
+                    rp++;
+                    if (!(*rp >= '0' && *rp <= '9'))
+                      return NULL;
+                  }
 		val = val * 10 + *rp++ - '0';
 		++n;
 	      }
diff --git a/time/tst-strptime2.c b/time/tst-strptime2.c
index c22967a93026..1c0958fc8bc0 100644
--- a/time/tst-strptime2.c
+++ b/time/tst-strptime2.c
@@ -13,16 +13,25 @@ static const struct
     { "1113472456 -1000", -36000 },
     { "1113472456 +10", 36000 },
     { "1113472456 -10", -36000 },
+    { "1113472456 +10:00", 36000 },
+    { "1113472456 -10:00", -36000 },
     { "1113472456 +1030", 37800 },
     { "1113472456 -1030", -37800 },
+    { "1113472456 +10:30", 37800 },
+    { "1113472456 -10:30", -37800 },
     { "1113472456 +0030", 1800 },
     { "1113472456 -0030", -1800 },
     { "1113472456  Z", 0 },
     { "1113472456 -1330", LONG_MAX },
     { "1113472456 +1330", LONG_MAX },
+    { "1113472456 -13:30", LONG_MAX },
+    { "1113472456 +13:30", LONG_MAX },
     { "1113472456 -1060", LONG_MAX },
     { "1113472456 +1060", LONG_MAX },
+    { "1113472456 -10:60", LONG_MAX },
+    { "1113472456 +10:60", LONG_MAX },
     { "1113472456  1030", LONG_MAX },
+    { "1113472456  10:30", LONG_MAX },
   };
 #define ntests (sizeof (tests) / sizeof (tests[0]))
 
-- 
2.1.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] time: ensure failing strptime() tests are reported correctly
  2015-01-27 15:20 [PATCH] time: ensure failing strptime() tests are reported correctly Vincent Bernat
  2015-01-27 16:02 ` [PATCH] time: in strptime(), make %z accept Z as a time zone Vincent Bernat
  2015-01-27 16:03 ` [PATCH] time: in strptime(), make %z accept [+-]HH:MM time zones Vincent Bernat
@ 2015-03-06 10:04 ` Mike Frysinger
  2015-03-06 11:02   ` Vincent Bernat
  2 siblings, 1 reply; 10+ messages in thread
From: Mike Frysinger @ 2015-03-06 10:04 UTC (permalink / raw)
  To: Vincent Bernat; +Cc: libc-alpha, Vincent Bernat

[-- Attachment #1: Type: text/plain, Size: 101 bytes --]

patch looks fine, but please write a ChangeLog entry.  i'll push this once 
you've done so.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] time: in strptime(), make %z accept Z as a time zone
  2015-01-27 16:02 ` [PATCH] time: in strptime(), make %z accept Z as a time zone Vincent Bernat
@ 2015-03-06 10:11   ` Mike Frysinger
  0 siblings, 0 replies; 10+ messages in thread
From: Mike Frysinger @ 2015-03-06 10:11 UTC (permalink / raw)
  To: Vincent Bernat; +Cc: libc-alpha, Vincent Bernat

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

On 27 Jan 2015 16:05, Vincent Bernat wrote:
> In ISO 8601, the timezone can be 'Z' instead of using
> digits. 2014-08-17T12:33:12+0000 is often expressed as
> 2014-08-17T12:33:12Z.

code wise, this looks good to me.  style problems though ...

> --- a/time/strptime_l.c
> +++ b/time/strptime_l.c
>
> -	     also specified.  */
> +	     also specified. 'Z' is equivalent to +0000. */

GNU style puts two spaces after the period

>  	  {
>  	    val = 0;
>  	    while (ISSPACE (*rp))
>  	      ++rp;
> +            if (*rp == 'Z')
> +              {
> +                rp++;
> +                break;
> +              }

indentation is incorrect -- you need to start with a tab

minor nit: this code would do ++rp instead of rp++
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] time: in strptime(), make %z accept [+-]HH:MM time zones
  2015-01-27 16:03 ` [PATCH] time: in strptime(), make %z accept [+-]HH:MM time zones Vincent Bernat
@ 2015-03-06 10:21   ` Mike Frysinger
  2015-03-06 11:06     ` Vincent Bernat
  2015-04-20 11:42     ` [BZ #17887][PATCH] " Vincent Bernat
  0 siblings, 2 replies; 10+ messages in thread
From: Mike Frysinger @ 2015-03-06 10:21 UTC (permalink / raw)
  To: Vincent Bernat; +Cc: libc-alpha, Vincent Bernat

[-- Attachment #1: Type: text/plain, Size: 2373 bytes --]

On 27 Jan 2015 16:20, Vincent Bernat wrote:
> In ISO 8601, +03:30 is a valid time zone. Currently, strptime() only
> parses it as a 2-digit time zone an believes this is +03:00. This change
> makes it accept a single semi-colon.

err, nowhere that i see here do you parse a semi-colon.  i guess you meant
"colon" ?

i think you're colliding with the fix for BZ #16141.  i'd prefer to merge
that first though, so you might want to rebase once that's done.

> This fix BZ #17887.

you should include [BZ #17887] in the subject line too

> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -3,6 +3,9 @@
>  	[BZ #17886]
>  	* time/strptime_l.c: Make %z accept Z as a valid time zone.
>  
> +	[BZ #17887]
> +	* time/strptime_l.c: Make %z accept [+-]HH:MM time zones.

each patch should get a standalone entry.  that means you'd add another 
date/name line above this.

also you should scope your changes a bit.  that means this would be:
	* time/strptime_l.c (__strptime_internal): .....

> +	  /* We recognize four formats: 1. if two digits are given,
> +	     these specify hours.  2. If fours digits are used,
> +	     minutes are also specified. 3. A semi-colon can be used
> +	     to separate the two groups of two digits (HH:MM). 4. 'Z'
> +	     is equivalent to +0000. */

needs two spaces after the periods

> @@ -765,8 +767,16 @@ __strptime_internal (rp, fmt, tmp, statep LOCALE_PARAM)
>  	      return NULL;
>  	    bool neg = *rp++ == '-';
>  	    int n = 0;
> -	    while (n < 4 && *rp >= '0' && *rp <= '9')
> +	    while (n < 4 &&
> +                   ((*rp >= '0' && *rp <= '9') ||
> +                    (*rp == ':' && n == 2)))
>  	      {
> +                if (*rp == ':')
> +                  {
> +                    rp++;
> +                    if (!(*rp >= '0' && *rp <= '9'))
> +                      return NULL;
> +                  }
>  		val = val * 10 + *rp++ - '0';
>  		++n;
>  	      }

indentation is broken -- needs to start with a tab

not exactly a new issue, but probably should be using isdigit() here

i think the loop might be cleaner if you didn't duplicate the checks.
i.e. something like this (ignoring style, and this is untested):
	while (n < 4) {
		if (n == 2 && *rp == ':')
			++rp;
		if (!isdigit (*rp))
			return NULL;
		val = val * 10 + *rp++ - '0';
		++n;
	}
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] time: ensure failing strptime() tests are reported correctly
  2015-03-06 10:04 ` [PATCH] time: ensure failing strptime() tests are reported correctly Mike Frysinger
@ 2015-03-06 11:02   ` Vincent Bernat
  2015-03-06 11:07     ` Mike Frysinger
  0 siblings, 1 reply; 10+ messages in thread
From: Vincent Bernat @ 2015-03-06 11:02 UTC (permalink / raw)
  To: libc-alpha; +Cc: Vincent Bernat

From: Vincent Bernat <vincent@bernat.im>

---
 ChangeLog            | 6 ++++++
 time/tst-strptime2.c | 5 +----
 time/tst-strptime3.c | 5 +----
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 420f351ee0d9..0eb779d91fce 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2015-03-06  Vincent Bernat  <vincent@bernat.im>
+
+	* time/tst-strptime2.c (do_test): Ensure failing tests are
+	reported correctly.
+	* time/tst-strptime3.c (do_test): Likewise.
+
 2015-03-06  Samuel Thibault  <samuel.thibault@inria.fr>
 
 	Fix aio_error thread-safety.
diff --git a/time/tst-strptime2.c b/time/tst-strptime2.c
index bcd6cc8be98f..a08e6d7cb7de 100644
--- a/time/tst-strptime2.c
+++ b/time/tst-strptime2.c
@@ -52,10 +52,7 @@ do_test (void)
 	}
     }
 
-  if (result == 0)
-    puts ("all OK");
-
-  return 0;
+  return result;
 }
 
 #define TEST_FUNCTION do_test ()
diff --git a/time/tst-strptime3.c b/time/tst-strptime3.c
index 75b57c192871..d53f51ee2244 100644
--- a/time/tst-strptime3.c
+++ b/time/tst-strptime3.c
@@ -48,10 +48,7 @@ do_test (void)
       result = 1;
     }
 
-  if (result == 0)
-    puts ("all OK");
-
-  return 0;
+  return result;
 }
 
 #define TEST_FUNCTION do_test ()
-- 
2.1.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] time: in strptime(), make %z accept [+-]HH:MM time zones
  2015-03-06 10:21   ` Mike Frysinger
@ 2015-03-06 11:06     ` Vincent Bernat
  2015-04-20 11:42     ` [BZ #17887][PATCH] " Vincent Bernat
  1 sibling, 0 replies; 10+ messages in thread
From: Vincent Bernat @ 2015-03-06 11:06 UTC (permalink / raw)
  To: libc-alpha

 ❦  6 mars 2015 05:21 -0500, Mike Frysinger <vapier@gentoo.org> :

> i think you're colliding with the fix for BZ #16141.  i'd prefer to merge
> that first though, so you might want to rebase once that's done.

OK, I am waiting for BZ #16141 to be pushed before pushing the patches
for BZ #17886 and BZ #17887 (since they are co-dependant).
-- 
Vincent Bernat — Vincent.Bernat@exoscale.ch
❬❱ http://www.exoscale.ch

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] time: ensure failing strptime() tests are reported correctly
  2015-03-06 11:02   ` Vincent Bernat
@ 2015-03-06 11:07     ` Mike Frysinger
  0 siblings, 0 replies; 10+ messages in thread
From: Mike Frysinger @ 2015-03-06 11:07 UTC (permalink / raw)
  To: Vincent Bernat; +Cc: libc-alpha, Vincent Bernat

[-- Attachment #1: Type: text/plain, Size: 21 bytes --]

thanks, merged
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [BZ #17887][PATCH] time: in strptime(), make %z accept [+-]HH:MM time zones
  2015-03-06 10:21   ` Mike Frysinger
  2015-03-06 11:06     ` Vincent Bernat
@ 2015-04-20 11:42     ` Vincent Bernat
  1 sibling, 0 replies; 10+ messages in thread
From: Vincent Bernat @ 2015-04-20 11:42 UTC (permalink / raw)
  To: libc-alpha

 ❦  6 mars 2015 04:21 -0500, Mike Frysinger <vapier@gentoo.org> :

> i think you're colliding with the fix for BZ #16141.  i'd prefer to merge
> that first though, so you might want to rebase once that's done.

It seems there is no more activity on #16141. What should I do?
-- 
Vincent Bernat — Vincent.Bernat@exoscale.ch
❬❱ http://www.exoscale.ch

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-04-20 11:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-27 15:20 [PATCH] time: ensure failing strptime() tests are reported correctly Vincent Bernat
2015-01-27 16:02 ` [PATCH] time: in strptime(), make %z accept Z as a time zone Vincent Bernat
2015-03-06 10:11   ` Mike Frysinger
2015-01-27 16:03 ` [PATCH] time: in strptime(), make %z accept [+-]HH:MM time zones Vincent Bernat
2015-03-06 10:21   ` Mike Frysinger
2015-03-06 11:06     ` Vincent Bernat
2015-04-20 11:42     ` [BZ #17887][PATCH] " Vincent Bernat
2015-03-06 10:04 ` [PATCH] time: ensure failing strptime() tests are reported correctly Mike Frysinger
2015-03-06 11:02   ` Vincent Bernat
2015-03-06 11:07     ` Mike Frysinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).