[libvirt] [PATCH v2 1/4] Add a read/write lock implementation

[Daniel P. Berrange]

On Mon, Jan 27, 2014 at 03:45:13PM -0700, Eric Blake wrote:
> On 01/27/2014 10:18 AM, Daniel P. Berrange wrote:
> > Add virRWLock backed up by a POSIX rwlock primitive
> > 
> > Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> > ---
> 
> > +int virRWLockInit(virRWLockPtr m)
> > +{
> > +    if (pthread_rwlock_init(&m->lock, NULL) != 0) {
> > +        errno = EINVAL;
> > +        return -1;
> 
> My concern from v1 still stands - this blindly overwrites non-EINVAL
> errors, and better would be:
> 
> int rc = pthread_rwlock_init(&m->lock, NULL);
> if (rc) {
>     errno = rc;
>     return -1;
> }

Ah, yes, it shuld match MutexInit in this way.

> > +void virRWLockDestroy(virRWLockPtr m)
> > +{
> > +    pthread_rwlock_destroy(&m->lock);
> 
> Likewise, it might be nice to add VIR_DEBUG messages when discarding a
> non-zero result, as a way to diagnose the programmer errors that caused
> that situation.

Well we don't do that logging for any of the other Destroy
calls in this threads code. If you think its useful then
we should do it everywhere perhaps ?

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|