Re: [libvirt] [PATCH v2 1/4] Add a read/write lock implementation
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(a)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 :|