On Sat, Jul 03, 2021 at 02:57:30PM +0200, René Scharfe wrote:
> We use our standard allocation functions and macros (xcalloc,
> ALLOC_ARRAY, REALLOC_ARRAY) in our version of khash.h. They terminate
> the program on error instead, so code that's using them doesn't have to
> handle allocation failures. Make this behavior explicit by turning
> kh_resize_ into a void function and removing the related unreachable
> error handling code.
Thanks, this looks good to me.
> SCOPE khint_t kh_put_##name(kh_##name##_t *h, khkey_t key, int *ret) \
> { \
> khint_t x; \
> if (h->n_occupied >= h->upper_bound) { /* update the hash table */ \
> if (h->n_buckets > (h->size<<1)) { \
> - if (kh_resize_##name(h, h->n_buckets - 1) < 0) { /* clear "deleted" elements */ \
> - *ret = -1; return h->n_buckets; \
> - } \
> - } else if (kh_resize_##name(h, h->n_buckets + 1) < 0) { /* expand the hash table */ \
> - *ret = -1; return h->n_buckets; \
> + kh_resize_##name(h, h->n_buckets - 1); /* clear "deleted" elements */ \
> + } else { \
> + kh_resize_##name(h, h->n_buckets + 1); /* expand the hash table */ \
> } \
I had to read this over again carefully, because the second "else-if"
makes the conversion less obvious. I think the original would have been
easier to read as:
if (something) {
if (do_option_one())
...error...
} else {
if (do_option_two())
...error...
}
instead of:
if (something) {
if (do_option_one())
...error...
} else if (do_option_two())
...error...
}
All of which is to say the conversion here is correct, and I think as a
bonus the resulting code is easier to follow. ;)
-Peff
