Re: [PATCH] time: in strptime(), make %z accept [+-]HH:MM time zones

[Mike Frysinger <vapier@gentoo.org>]

[-- 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 --]