[zeromq-dev] CZMQ: Error checking of z*_new() calls in other z*_new() functions

Pieter Hintjens ph at imatix.com
Thu Sep 4 11:51:27 CEST 2014


I'd prefer assertions in most places. The problem is that adding full
error handling on every allocation creates more complex code and
overall increases the risk of other errors.

There are a few places where it's worth catching heap exhaustion, and
that is on message handling, ans you'll see this done in zmsg and
zframe for instance.

However for the most part I don't think application developers are
competent to deal with out of memory situations correctly, and the
best strategy is to fail loudly and force the machine or architecture
to be fixed.

So we can start by asserting inside constructors, and then fix those
constructors we want to, to return NULL on failures.

On Thu, Sep 4, 2014 at 11:37 AM, Goswin von Brederlow <goswin-v-b at web.de> wrote:
> Hi,
>
> while playing with the zlist API I had to go through many source files
> adjusting things here and there. One thing I noticed is that various
> z*_new() functions don't check for errors on other z*_new() calls they
> make (or any call that might fail). For example:
>
> zdir_t *
> zdir_new (const char *path, const char *parent)
> {
>     zdir_t *self = (zdir_t *) zmalloc (sizeof (zdir_t));
>     if (parent) {
>         self->path = (char *) malloc (strlen (path) + strlen (parent) + 2);
>         sprintf (self->path, "%s/%s", parent, path);
>     }
>     else
>         self->path = strdup (path);
>     self->files = zlist_new ();
>     self->subdirs = zlist_new ();
> ...
>
> The malloc can fail, the strdup can fail, zlist_new can fail. None of
> those are checked or asserted.
>
> Is that check missing there or was it left out purposefully, letting
> the code crash & burn later when allocations fail?
>
>
> Other classes do check though. For example:
>
> zhash_t *
> zhash_new (void)
> {
>     zhash_t *self = (zhash_t *) zmalloc (sizeof (zhash_t));
>     if (self) {
>         self->prime_index = INITIAL_PRIME;
>         self->chain_limit = INITIAL_CHAIN;
>         size_t limit = primes [self->prime_index];
>         self->items = (item_t **) zmalloc (sizeof (item_t *) * limit);
>         if (!self->items)
>             zhash_destroy (&self);
>     }
>     return self;
> }
>
> Here the function will return NULL if self->items couldn't be allocated.
> Imho every class should do that.
>
> MfG
>         Goswin
> _______________________________________________
> zeromq-dev mailing list
> zeromq-dev at lists.zeromq.org
> http://lists.zeromq.org/mailman/listinfo/zeromq-dev



More information about the zeromq-dev mailing list