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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 31 10:01:25 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 57462: use memset
https://bugs.webkit.org/attachment.cgi?id=57462&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    // Make sure it's not the same value as PTHREAD_MUTEX_INITIALIZER.
> +    memset(&m_mutex, 0, sizeof(pthread_mutex_t));

If we are doing this memset, I think it's better style to call sizeof(m_mutex)
here instead of sizeof(pthread_mutex_t), because that way you can see looking
at this line of code that it's correct without having to look up the type of
m_mutex to double check that it's a pthread_mutex_t.

As I said in my previous comment in this bug, this comment and code are not
needed, and I am almost certain the comment reflects a misunderstanding on your
part. I don't want to confuse future programmers working on this code.

Initializing to zero does have a small good effect. It gives us more
predictable behavior if pthread_mutex_init fails. Both the old code path and
the new code path have the same failing in that they don't check the error
return of pthread_mutex_init. But this issue is not specific to this new code
path and applies just as much to the PTHREAD_MUTEX_NORMAL ==
PTHREAD_MUTEX_DEFAULT case.

Despite the fact that this patch is otherwise just fine, I'm going to say
review- because I don't want to check in a mysterious comment or unneeded code.


The only difference between the two branches of the #if should be the
attributes passed in to pthread_mutex_init.


More information about the webkit-reviews mailing list