[Webkit-unassigned] [Bug 34409] Use WTF::getLocalTime instead of localtime_r in FTPDirectoryDocument

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Feb 2 10:40:17 PST 2010


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
  Attachment #47949|review?, commit-queue?      |review+, commit-queue+
               Flag|                            |

--- Comment #5 from Darin Adler <darin at apple.com>  2010-02-02 10:40:17 PST ---
(From update of attachment 47949)
Looks good.

> -    localtime_r(&now_t, &now);
> +    WTF::getLocalTime(&now_t, &now);

The correct way to do this with WTF is to add:

    using WTF::getLocalTime;

to the bottom of the CurrentTime.h header. And not include the "WTF::" prefix

I won’t hold up this patch just for that reason, but I hope you are willing to
tackle this rather than leaving it for someone else.

>  #if PLATFORM(QT) && defined(Q_WS_WIN32)
> -// Defined in FTPDirectoryDocument.cpp.
> -struct tm gmtimeQt(const QDateTime &input);
> +
> +// Replacement for localtime_r() which is not available on MinGW.
> +// We use this on all of Qt's platforms for portability.

The comment is now incorrect with the function in its new location. This is
only used for Win32 in this file.

It's incorrect to call this a "replacement for localtime_r" here; it's a
replacement for gmtime_r.

This is not the normal portability approach for WebKit. We typically put these
platform dependencies in WTF or the platform directory, not directly in the
affected file. The WebKit approach to this would be to create a replacement for
gmtime_r in a WTF header named something like TimeExtras.h or DateExtras.h
rather than having it here in this source file.

> +struct tm gmtimeQt(const QDateTime &input)

Style comment on code that was moved: The "&" character should be next to the
type, not the argument name. Not sure why the style bot let you get away with

> +    const QDate date(input.date());

> +    const QTime time(input.time());

We normally don't use const like this on local variables in WebKit code. There
are lots of local variables that could be marked const, and we decided to mark
none of them rather than trying to mark all of them.

r=me despite these comments

Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

More information about the webkit-unassigned mailing list