[Webkit-unassigned] [Bug 9651] MSVCRT time functions are too strict

[bugzilla-daemon at opendarwin.org]

http://bugzilla.opendarwin.org/show_bug.cgi?id=9651





------- Comment #9 from bjoern.graf at gmail.com  2006-07-11 22:37 PDT -------
(In reply to comment #8)
> +    if (day < dfy) {
> +        // The first day of year is beyond day, so we have right year + 1
> +        year -= 1;
>      }
> 
> no need for braces

The WebKit style guidelines are so contrary to my own that I sometimes miss to
follow the right ones...

> -  static double time_tMin = (time_tIsSigned ? - (double)(1ULL << (8 *
> sizeof(time_t) - 1)) : 0);
> +  if (secs < 0 || secs > time_tMax) {
> 
> If this is removed, so that mean that time_tIsSigned is not needed any more?

No. time_tIsSigned is used in time_tMax. Actually it might be better to limit
time_tMax to the defined valid range of the time functions (1970 - 2038) like
it is done in makeTime().

> -  char timebuffer[bufsize];
> +  wchar_t timebuffer[bufsize];
> 
> What's the rationale for this change?

Using wcsftime instead of strftime to save the conversion to Unicode in the
code line next to it.

> +      size_t len = WTF::wcsftime(timebuffer, bufsize, L"%c", &t);
> +      return jsString(UString((UChar*)timebuffer, len));
> 
> This assumes that wchar_t is the same as UChar, but that's not going to be true
> on platforms other than Windows.
>

This is interesting: there is no way to do a wcsftime(str.getBuffer(bufsize),
bufsize, ...) on non-Windows systems then? At least it explains why there is no
UString c'tor that accepts a pointer to wchar_t. I guess I need to revert to
strftime then.

> +    //FIXME: Cache the result per thread
> 
> We put spaces after // comments.

VC++ has the nice little feature showing comments like TODO and FIXME in its
task list. The drawback is that it does not allow a space between the comment
and the token. Anyway, it looks like I forgot to transform them.

> +#ifndef _DATEEXTRAS_H_
> 
> Should not use a leading underscore -- that's reserved for the compiler and
> library.

I always thought that two leading underlines are reserved for the compiler
only. Should it be changed to WTF_DATE_EXTRAS_H or KXMLCORE_DATE_EXTRAS_H to
match the other header files in WTF?

> +#if HAVE(SYS_TIME_H)
> +#if PLATFORM(WIN)
> 
> Need to include "Platform.h" first if a header file is going to use these
> macros.

Neither AlwaysInline.h nor FastMalloc.h include Platform.h so I assumed it was
save to not include platform.h.


-- 
Configure bugmail: http://bugzilla.opendarwin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.