Re: [gpsd-dev] [PATCH] Add Windows versions of time related functions.

[Robert Norris]

> On Fri, 18 Mar 2016 19:16:55 -0700
> Hal Murray <address@hidden> wrote:
>
>> address@hidden said:
>>> sscanf() is very touchy and segfaults when you glance at it
>>> sideays.
>
>> Does it segfault trying to read the input string or trying to write a
>> string result?
>
> 2 problems.
>
> First, if the input to sscanf() is not exactly what sscanf() is
> expecting, it often segfaults.
>
> Second, getting the proper types matched up for the destination
> pointers is really hard to do over multiple OS.
>
> Check out how NetBSD does it:
> http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/time/strptime.c?rev=HEAD
>
> No sscanf() in there. That is likely overkill for the needed functions.
>
>> For simple fixed format strings, is it possible to use sscanf with a
>> safe format string? If so, is it possible to automatically verify
>> that the format string is safe?
>
> I've never seen a bullet proof use of sscanf() in the wild. It
> certainly caused problems for gpsd in the past. Once you do all the
> work to validate the sscanf() input you have done more work than if you
> just parsed it yourself.
>
> The string to be parsed is pretty simple: "%u-%u-%uT%u:%u:%lf"
>
> Just split up the string into 6 parts, using dash, T and colon
> as delimiters. Then just use atoi() on the 6 parts, and range check
> the results. Easy, and hard to break.

OK thanks for the advice.

I've rewritten the patch using strtok() instead of sscanf().

Is this the right way to go ? (or should I manually process each character such 
as how strptime() does?)

Although I've just re-read your message - so I need to add some range checking 
before I post the new version.

Thus far it's looking like this:

#else
    /* Fallback for systems without strptime (i.e. Windows)
       This is a simplistic conversion for iso8601 strings only,
       rather than embedding a full copy of strptime() that handles all formats 
*/
    double sec;
    char *isotime_tokenizer = strdup(isotime);
    char *pch = strtok(isotime_tokenizer, "-T:");
    int token_number = 0;
    while ( pch != NULL ) {
      token_number++;
      switch (token_number) {
        case 1: tm.tm_year = atoi(pch) - 1900; break; // Adjust to tm year
        case 2: tm.tm_mon = atoi(pch) - 1; break; // Month indexing starts from 
zero
        case 3: tm.tm_mday = atoi(pch); break;
        case 4: tm.tm_hour = atoi(pch); break;
        case 5: tm.tm_min = atoi(pch); break;
        case 6:
      sec = safe_atof(pch);
      tm.tm_sec = (unsigned int)sec; // Truncate to get integer value
          usec = sec - (unsigned int)sec; // Get the fractional part (if any)
      break;
        default: break;
      }
      pch = strtok(NULL, "-T:");
    }
    free(isotime_tokenizer);
    (void)mktime(&tm); // Normalizes tm so that tm_yday is set
#endif