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

[Jincheng Miao]

----- Original Message -----
> 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.

OK, it's fine to drop them, and thanks for your review.

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