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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> > ---
> 
> > +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 :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]