[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