Re: [Libevent-users] Missing error checking

[Nick Mathewson <nickm@xxxxxxxxxxxxx>]

On Mon, Jun 6, 2011 at 5:29 PM, Gilad Benjamini <gbenjamini@xxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.