[vlc-devel] [Patch] Adding an helper to convert iso8601 durations to seconds

[Tobias Güntner]

Am 30.11.2011 11:05, schrieb Hugo Beauzée-Luyssen:
> +/*
> +  Decodes a duration as defined by ISO 8601
> +  http://en.wikipedia.org/wiki/ISO_8601#Durations
> +  @param str A null-terminated string to convert

Perhaps a short example of a valid input string would be useful, so
readers can see at first glance what kind of data this function expects,
without looking it up.

> +  @return: The duration in seconds. 0 if an error occured.

This function usually does not return 0 if the input is invalid.

> + */
> +uint64_t str_iso8601_duration_to_seconds( const char *psz_duration )
> +{
> +    bool        timeDesignatorReached = false;
> +    double      mul = 0;

I think an integer should be fine.

> +    mtime_t     res = 0;

Shouldn't this type and the return type be the same?

> +
> +    if ( *psz_duration++ != 'P' )
> +        return 0;
> +    for ( const char* nptr = psz_duration; *psz_duration; ++psz_duration )
> +    {
> +        switch( *psz_duration )
> +        {
> +            case 'M':
> +            {

I find this a little confusing because letters normally follow numbers
in the input, yet you somehow parse letters first. ;)

> +                //M can mean month or minutes, if the 'T' flag has been reached.
> +                //We don't handle months though.
> +                if ( timeDesignatorReached == true )
> +                    mul = 60.0;
> +                break ;
> +            }
> +            case 'Y':
> +            case 'W':
> +                break ; //Don't handle this duration.

Maybe return an error here?

> +            case 'D':
> +                mul = 86400.0;
> +                break ;
> +            case 'T':
> +                timeDesignatorReached = true;
> +                break ;
> +            case 'H':
> +                mul = 3600.0;
> +                break ;
> +            case 'S':
> +                mul = 1.0;
> +                break ;
> +            default:
> +                continue ;
> +        }
> +        //It means we went trough some digits. So we must compute a number
> +        if ( nptr != psz_duration )
> +        {
> +            double tmp = strtold( nptr, NULL );

I think you can achieve a slightly cleaner design if you utilize the
second parameter of that function. The strtoX functions store the
address of the first invalid character in there, so all you have to do
is basically:

1. Call strtoul(psz_duration, &endptr, 10)
2. Check the character at *endptr
3. Repeat with psz_duration = endptr + 1

> +            res += (uint64_t)(tmp * mul);
> +            nptr = psz_duration;
> +        }
> +        nptr++;
> +    }
> +    return res;
> +}

Regards,
Tobias