[libvirt] [PATCH V2 2/3] add error report for virMutexInit virRWLockInit and virCondInit

[Daniel P. Berrange]

On Mon, Jul 21, 2014 at 06:13:36PM +0800, Jincheng Miao wrote:
> Implement vir{Mutex,RWLock,Cond}InitInternal functions which have
> related error report when failure, and vir{Mutex,RWLock,Cond}Init
> as macros. So that the caller no longer to print error message
> explicitly.
> 
> Signed-off-by: Jincheng Miao <jmiao at redhat.com>
> ---
>  src/libvirt_private.syms |  7 +++---
>  src/util/virthread.c     | 56 +++++++++++++++++++++---------------------------
>  src/util/virthread.h     | 25 +++++++++++++++++----
>  3 files changed, 49 insertions(+), 39 deletions(-)
> 

>  
> +#define VIR_THREAD_ERR_EXIT(str) do {                                   \
> +        errno = ret;                                                    \
> +        virReportSystemErrorFull(VIR_FROM_NONE, errno,                  \
> +                                 filename, funcname, linenr,            \
> +                                 "%s", _(str));                         \
> +        return -1;                                                      \
> +    } while (0)

[snip]

> -int virCondInit(virCondPtr cond)
> +int virCondInitInternal(virCondPtr cond, const char *filename,
> +                        const char *funcname, size_t linenr)
>  {
> -    int ret;
> -    if ((ret = pthread_cond_init(&cond->cond, NULL)) != 0) {
> -        errno = ret;
> -        return -1;
> -    }
> +    int ret = pthread_cond_init(&cond->cond, NULL);
> +    if (ret != 0)
> +        VIR_THREAD_ERR_EXIT("unable to init Condition");
> +
>      return 0;
>  }

[snip]

> -int virCondInit(virCondPtr cond) ATTRIBUTE_RETURN_CHECK;
> +int virCondInitInternal(virCondPtr cond, const char *filename,
> +                        const char *funcname, size_t linenr)
> +    ATTRIBUTE_RETURN_CHECK;
> +# define virCondInit(cond)                                      \
> +    virCondInitInternal(cond, __FILE__, __FUNCTION__, __LINE__)
> +

I can see what you're trying todo here by passing in __FILE__,
__FUNCTION__, __LINE__ through from the calling site, but I don't
really think this is beneficial and is not the way we deal with
error reporting in any other similar helper APIs. IMHO just
remove all this passing around of source location and let it do
the default thing. In fact the default behaviour is probably
better since if we want to search for any thread related error
messages, we can query the journald for the virthread.c file
directly.

Regards,
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 :|