From: Haitao Li <lihaitao@gmail.com>
Date: Sat, 10 Sep 2011 08:29:06 GMT
Subject: Re: [PATCH v3] date.c: Support iso8601 timezone formats
Message-ID: <CABk5KLi-Q+qWokZFJGF+uDvkxGJVvFO4y38H8B1sO6SnHOWYpA@mail.gmail.com>
In-Reply-To: <7vd3f9ve9m.fsf@alter.siamese.dyndns.org>

>
> Also, I do not quite understand why the match_tz() logic needs to be that
> long.
>
> Wouldn't something like this patch (on top of yours) easier to follow?

I was wrong about accepting one digit in hours or minutes. And yes
your version is conciser and easier to follow. Thanks!

>
>  date.c |   50 +++++++++++++++++++++-----------------------------
>  1 files changed, 21 insertions(+), 29 deletions(-)
>
> diff --git a/date.c b/date.c
> index f8722c1..6079b1a 100644
> --- a/date.c
> +++ b/date.c
> @@ -551,44 +551,36 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt
>
>  static int match_tz(const char *date, int *offp)
>  {
> +       int min;
>        char *end;
> -       int offset = strtoul(date+1, &end, 10);
> -       int min, hour;
> -       int n = end - date - 1;
> +       int hour = strtoul(date + 1, &end, 10);
> +       int n = end - (date + 1);
>
> -       /*
> -        * ISO8601:2004(E) allows time zone designator been separated
> -        * by a clone in the extended format
> -        */
> -       if (*end == ':') {
> -               if (isdigit(end[1])) {
> -                       hour = offset;
> -                       min = strtoul(end+1, &end, 10);
> -               } else {
> -                       /* Mark as invalid */
> -                       n = -1;
> -               }
> -       } else {
> -               if (n == 1 || n == 2) {
> -                       /* Only hours specified */
> -                       hour = offset;
> -                       min = 0;
> -               } else {
> -                       hour = offset / 100;
> -                       min = offset % 100;
> -               }
> +       if (n == 4) {
> +               /* hhmm */
> +               min = hour % 100;
> +               hour = hour / 100;
> +       } else if (n != 2) {
> +               min = 99; /* random crap */
> +       } else if (*end == ':') {
> +               /* hh:mm? */
> +               min = strtoul(end + 1, &end, 10);
> +               if (end - (date + 1) != 5)
> +                       min = 99; /* random crap */
>        }
>
>        /*
> -        * Don't accept any random crap.. We might want to check that
> -        * the minutes are divisible by 15 or something too. (Offset of
> +        * Don't accept any random crap. Even though some places have
> +        * offset larger than 12 hours (e.g. Pacific/Kiritimati is at
> +        * UTC+14), there is something wrong if hour part is much
> +        * larger than that. We might also want to check that the
> +        * minutes are divisible by 15 or something too. (Offset of
>         * Kathmandu, Nepal is UTC+5:45)
>         */
> -       if (n > 0 && min < 60) {
> -               offset = hour*60+min;
> +       if (min < 60 && hour < 24) {
> +               int offset = hour * 60 + min;
>                if (*date == '-')
>                        offset = -offset;
> -
>                *offp = offset;
>        }
>        return end - date;
>