From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS53758 23.128.96.0/24 X-Spam-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id 4F6241F8C7 for ; Sun, 4 Jul 2021 09:05:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229492AbhGDJIT (ORCPT ); Sun, 4 Jul 2021 05:08:19 -0400 Received: from cloud.peff.net ([104.130.231.41]:41126 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229744AbhGDJIS (ORCPT ); Sun, 4 Jul 2021 05:08:18 -0400 Received: (qmail 8050 invoked by uid 109); 4 Jul 2021 09:05:43 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sun, 04 Jul 2021 09:05:43 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 5967 invoked by uid 111); 4 Jul 2021 09:05:45 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Sun, 04 Jul 2021 05:05:45 -0400 Authentication-Results: peff.net; auth=none Date: Sun, 4 Jul 2021 05:05:42 -0400 From: Jeff King To: =?utf-8?B?UmVuw6k=?= Scharfe Cc: Git List , Junio C Hamano , Eric Wong , =?utf-8?B?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason Subject: Re: [PATCH v2] khash: clarify that allocations never fail Message-ID: References: <3778fb28-ed19-e90e-216a-d29d72305155@web.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <3778fb28-ed19-e90e-216a-d29d72305155@web.de> Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org 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