[webkit-reviews] review denied: [Bug 39893] Explicitly use PTHREAD_MUTEX_NORMAL to create pthread mutex : [Attachment 57417] Updated

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat May 29 21:01:14 PDT 2010


Darin Adler <darin at apple.com> has denied Yong Li <yong.li.webkit at gmail.com>'s
request for review:
Bug 39893: Explicitly use PTHREAD_MUTEX_NORMAL to create pthread mutex
https://bugs.webkit.org/show_bug.cgi?id=39893

Attachment 57417: Updated
https://bugs.webkit.org/attachment.cgi?id=57417&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Patch looks generally good, but I have some concerns.

> +    // Make sure it's not the same value as PTHREAD_MUTEX_INITIALIZER.

This comment makes no sense. Why is it important to make sure that something is
not the same value as PTHREAD_MUTEX_INITIALIZER? How is that relevant at all?

> +    // Static variables are always zeroed.
> +    static pthread_mutex_t s_zeroedMutex;
> +    m_mutex = s_zeroedMutex;

It seems really strange to do it that way. If you just want to zero something
out, the best way I can think of is:

    memset(&m_mutex, 0, sizeof(m_mutex));

No need for a global variable.

But why does m_mutex need to be initialized twice? Doesn't pthread_mutex_init
do a complete job? Why the need to initialize it before calling that?

review- because I don't think we should land this with the extra unneeded code
and the mysterious comment


More information about the webkit-reviews mailing list