4 ms·
In your example, there are two different places where you potentially destroy the intstack. It's not particularly bad because there are only three possible res
by boomlinde 3y ago
In your example, there are two different places where you potentially destroy the intstack.
It's not particularly bad because there are only three possible resource allocation failures in your example, but it still illustrates a problem that will be worse when applying the approach as a general strategy. Consider a case where regex_canonical_equivalent_with_structures needed two stacks and two maps:
intstack_t stack_a = intstack_create(256);
if (!intstack_valid(stack_a)) {
return regex_compare_out_of_memory;
}
intstack_t stack_b = intstack_create(256);
if (!intstack_valid(stack_b)) {
intstack_destroy(stack_a);
return regex_compare_out_of_memory;
}
intmap_t map_a = intmap_create(16);
if (!intmap_valid(map_a)) {
intstack_destroy(stack_a);
intstack_destroy(stack_b);
return regex_compare_out_of_memory;
}
intmap_t map_b = intmap_create(16);
if (!intmap_valid(map_b)) {
intstack_destroy(stack_a);
intstack_destroy(stack_b);
intmap_destroy(map_a);
return regex_compare_out_of_memory;
}
regex_compare_t res =
regex_canonical_equivalent_with_structures(cache, x, y, &stack_a, &stack_b &map_a, &map_b);
intstack_destroy(stack_a);
intstack_destroy(stack_b);
intmap_destroy(map_a);
intmap_destroy(map_b);
return res;
}
You now have a 5-tuple maintenance problem and the code isn't particularly readable. I call it a "wet staircase"; it's not DRY, it's easy to slip and it gets taller for each step :)
Consider the equivalent using a simple goto escape hatch:
regex_compare_t regex_canonical_equivalent(regex_cache_t *cache,
stringtable_index_t x,
stringtable_index_t y) {
if (x.value == y.value) {
return regex_compare_equal;
}
intstack_t stack_a = intstack_create(256);
if (!intstack_valid(stack_a)) goto out_of_memory;
intstack_t stack_b = intstack_create(256);
if (!intstack_valid(stack_b)) goto out_of_memory;
intmap_t map_a = intmap_create(16);
if (!intmap_valid(map_a)) goto out_of_memory;
intmap_t map_b = intmap_create(16);
if (!intmap_valid(map_b)) goto out_of_memory;
return regex_canonical_equivalent_with_structures(cache, x, y, &stack_a, &stack_b &map_a, &map_b);
out_of_memory:
if (intstack_valid(stack_a)) intstack_destroy(stack_a);
if (intstack_valid(stack_b)) intstack_destroy(stack_b);
if (intmap_valid(map_a)) intmap_destroy(map_a);
if (intmap_valid(map_b)) intmap_destroy(map_b);
return regex_out_of_memory;
}
Or if you add "destroyvalid" methods to your data structures,
out_of_memory:
intstack_destroyvalid(stack_a);
intstack_destroyvalid(stack_b);
intmap_destroyvalid(map_a);
intmap_destroyvalid(map_b);
return regex_out_of_memory;
}
- JonChesterfield 3y agoSure, the scaling is definitely better with the goto approach. Goto will probably codegen better too (consider the destructors being inlined). Separating allocation from whatever one is doing with the resources is usually still a win though - it decouples the cases you're checking when looking through the code.