Re: [libvirt] [PATCH v4 RESEND 1/4] threadpool impl

On Thu, Dec 02, 2010 at 03:26:57PM +0800, Hu Tao wrote: > +virThreadPoolPtr virThreadPoolNew(size_t minWorkers, > + size_t maxWorkers, > + virThreadPoolJobFunc func, > + void *opaque) > +{ > + virThreadPoolPtr pool; > + size_t i; > + > + if (minWorkers > maxWorkers) > + minWorkers = maxWorkers; > + > + if (VIR_ALLOC(pool) < 0) { > + virReportOOMError(); > + return NULL; > + } > + > + pool->jobList.head = NULL; > + pool->jobList.tail = &pool->jobList.head; > + > + pool->jobFunc = func; > + pool->jobOpaque = opaque; > + > + if (virMutexInit(&pool->mutex) < 0) > + goto error; > + if (virCondInit(&pool->cond) < 0) > + goto error; > + if (virCondInit(&pool->quit_cond) < 0) > + goto error; > + > + if (VIR_ALLOC_N(pool->workers, minWorkers) < 0) > + goto error; > + > + pool->maxWorkers = maxWorkers; > + for (i = 0; i < minWorkers; i++) { > + if (virThreadCreate(&pool->workers[i], > + true, > + virThreadPoolWorker, > + pool) < 0) { > + virThreadPoolFree(pool); > + return NULL; > + } > + pool->nWorkers++; > + } > + > + return pool; > + > +error: > + VIR_FREE(pool->workers); > + ignore_value(virCondDestroy(&pool->quit_cond)); > + ignore_value(virCondDestroy(&pool->cond)); > + virMutexDestroy(&pool->mutex); > + return NULL; > +} This is leaking 'pool' on error. IMHO it is preferrable to just call virThreadPoolDestroy, otherwise anytime we add another field to virThreadPoolPtr struct, we have to consider updating 2 cleanup paths.