I have found the following approach relatively neat, and I think it
demonstrates an acceptable use of goto:
Container* container_new(void)
{
Container* c = container_alloc();
if (!c) goto fail_c;
c->res1 = resoure1_new();
if (!c->res1) goto fail_res1;
c->res2 = resource2_new();
if (!c->res2) goto fail_res2;
...
c->res_n = resource_n_new();
if (!c->res_n) goto fail_res_n;
return c;
fail_res_n:
...
resource2_destroy(c->res2);
fail_res2:
resource1_destroy(c->res1);
fail_res1:
container_dealloc(c);
fail_c:
return NULL;
}
The good thing with this approach is that the code is quite clean and
all error handling is explicit. On the other hand, the error handling
may then duplicate some code with a container_destroy function that
would free a Container and its contents once they were all
successfully allocated. I think this is a reasonable tradeoff, though.
Lauri
I've used this idea and it works fine in a straight-line procedure,
but doesn't hold up if allocation ends up being conditional. At that
point it just falls apart.
Over years I've fallen into this strategy:
{
Declarations
Initializers: Put each data structure in a known state where no
resources have been allocated.
Setup: Assign resources data structures. If any part of an
allocation fails, return it to the initialized state. Allocators can
(and often should) be called lazily: just before the respective
structure is used.
Uses: Use the data structures. The uses can employ an
is_allocated() predicate for each data structure to decide if they can
run successfully. Any kind of abort does a goto cleanup;
cleanup:
Clearers: Return each data structure to its initialized state.
}
The secret to happiness is the invariants: (1) init_ is always called
exactly once just after declaration. (2) Setup is idempotent. You
can call it repeatedly, potentially with different parameters, and
it's no harm no foul. (3) Cleanup is also idempotent. If a previous
setup_ succeeded, it releases held resources and returns all to the
init_ state. Additional calls do nothing.
If you were implementing a vector data type in this scheme, it would
be:
typedef struct float_vector_s {
unsigned n_elts;
float *elts;
} FLOAT_VECTOR;
void init_float_vector(FLOAT_VECTOR *v)
{
v->n_elts = ~0u; // "initialized not allocated"
v->elts = NULL;
}
void clear_float_vector(FLOAT_VECTOR *v)
{
free(v->elts);
// clear_ (almost) always ends with init_
init_float_vector(v);
}
void setup_float_vector(FLOAT_VECTOR *v, unsigned n_elts)
{
// Setup (almost) always clears first.
clear_float_vector(v);
if (n_elts > 0) {
v->elts = malloc(n_elts * sizeof *v->elts);
if (v->elts) v->n_elts = n_elts;
}
else v->n_elts = 0; // 0-vectors are okay...
}
int is_float_vector_setup_ok(FLOAT_VECTOR *v)
{
return v->n_elts != ~0u;
}
Now in use it looks like this:
FLOAT_VECTOR v[1]; // 1-element array saves &'s.
init_float_vector(v); // just once!
// stuff
if (I need a vector) {
setup_float_vector(v, 100);
// We can test success.
if (is_float_vector_setup_ok(v)) {
// oops need more space. No problem.
setup_float_vector(v, 200);
// abort if space couldn't be found
if (!is_float_vector_setup_ok(v)) go cleanup;
}
// something might go wrong. No problem...
if (bad stuff happens) goto cleanup;
// If you need the space back here, just clear it
clear_float_vector(v);
// something else might go wrong. It's no problem
// that v is already clear.
if (bad stuff happens) goto cleanup;
}
cleanup:
clear_float_vector(v); // Idempotent, so an extra call doesn't
matter.
... and all the other clear_ routines are called here.
Note that this just generalizes the free(NULL) mechanism James Kuyper
mentioned.
