----- 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@xxxxxxxxxx> > > --- > > 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 :| > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list