> =====================================================
 > REVIEWER: [EMAIL PROTECTED]
 > WEBREV:   http://cr.grommit.com/~zf162725/cr_0308/
 > FILES:    Misc
 > NOTES:    Description of feedbacks:
 >            ACCEPT       Request accepted
 >            REJECT       Request rejected
 >            EXPLAIN      Explanation given
 >            DISCUSS      Request requires further discussion to resolve
 >            DEFER        Request deferred (e.g. because work is out-of-scope)
 > =====================================================
 > 
 > usr/src/cmd/cmd-inet/usr.lib/wpad/driver.h
 > usr/src/cmd/cmd-inet/usr.lib/wpad/eloop.h
 > usr/src/cmd/cmd-inet/usr.lib/wpad/wpa_enc.h
 > 
 > No comments.
 > 
 > usr/src/cmd/cmd-inet/usr.lib/wpad/driver_wifi.c
 > wpa_driver_wifi_get_bssid() line 64:
 > (void) pthread_mutex_lock(&gbuf_lk);
 > in case pthread_mutex_lock failed, it's better to use:
 > assert(pthread_mutex_lock(&gbuf_lk) == 0));
 > Please correct other usage of pthread_mutex_lock() in the file
 > 
 > | ACCEPT

I disagree with this code review comment.  assert() must *never* surround
code that is required for correctness, since it will be compiled out when
built non-DEBUG.  That is, if -DNDEBUG is defined, this assert() will go
away along with the pthread_mutex_lock() inside it, and the lock will not
be acquired.

This change needs to be undone.  (As an aside, it's perfectly normal to
ignore the return value from pthread_mutex_lock(), since it can never fail
in normal operation.)
 
-- 
meem
_______________________________________________
networking-discuss mailing list
[email protected]

Reply via email to