On Mon, Jun 6, 2011 at 5:29 PM, Gilad Benjamini <gbenjam...@juniper.net> wrote: > Hi, > Here are a few cases where error checking is missing. Not knowing the code, I > can mostly just point to the issues without suggesting fixes. > > - evthread_set_condition_callbacks - The first half of the function assumes > cbs might > be NULL. The second half references cbs without validation
Hmmm. I think that the allowing NULL-cbs is an issue here. Generally, it is a really bad idea to change the threading callbacks once libevent is running, or to disable threading entirely. I think that instead of checking for "NULL" here, we should maybe disallow setting the threading or condition callbacks more than once. What do other people think? Is there a valid use-case here that I'm missing? > - evbuffer_overlapped_new - mm_calloc return value is not checked Thanks; should be fixed in 89d5e09e4d3a4666bf > - event_base_free - All the code that follows the call to EVUTIL_ASSERT > assumes that base is non NULL. Assertion are useful for a debug environment, > but IMHO are not a valid tool for production code, especially in an > external-facing function Turned that into a non-assert check in 09fe97da3b0dc. It's a warn now, since it's almost certainly a programming error to ever call event_base_free(NULL) when there is no current_base set. > - A similar approach exists in GET_IO_SLOT_AND_CTOR; _ent is allocated and > the assertion is supposed to cover the case of a failure. Thanks; should be fixed in 89d5e09e4d3a4666bf cheers, -- Nick *********************************************************************** To unsubscribe, send an e-mail to majord...@freehaven.net with unsubscribe libevent-users in the body.
