From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3CE67C43334 for ; Sat, 9 Jul 2022 19:23:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229515AbiGITX0 (ORCPT ); Sat, 9 Jul 2022 15:23:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229481AbiGITXZ (ORCPT ); Sat, 9 Jul 2022 15:23:25 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 66C7515A10 for ; Sat, 9 Jul 2022 12:23:24 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 0324DB806A0 for ; Sat, 9 Jul 2022 19:23:23 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3051DC3411C; Sat, 9 Jul 2022 19:23:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1657394601; bh=0D6oFHqU+zFv3pjPrUJqqWWAIeXMFm2WwgHxERu24yQ=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=DFz0q9ylgzaCjgQTf6icDW9WBR7peMlE6kjaIl8AYDkSAMwzeO+ryhT2GK093j1RJ JqCHbYqHQHHwh3ZlMxWfdw5PJY1DOQpdjqIZVPFaNfcrT4+qgrOyKkFAGikcardzHw bBx/7X0V18Xhn6byaxbvY+1nUXQOeGtemaig7i4OeSAPIXgdqedbCT9dqqYRbHMVkR qAJ6zpfYkoTfAImoeld/Yt3yjcCN3ohY1H0WpVs/5m3bpiF9TZ3O3g3k5m0w+UGk/3 0sQ5etP0WkMMLILGPL/TC6dN/d4FvqNCV4XVC75XzSz6No1tMwln47fBXkT3Qjg/en kNo2lG3SUheQQ== Date: Sat, 9 Jul 2022 12:23:20 -0700 From: Jakub Kicinski To: David Lamparter Cc: netdev@vger.kernel.org, "David S. Miller" , Eric Dumazet , Nikolay Aleksandrov , David Ahern Subject: Re: [PATCH net-next v5] net: ip6mr: add RTM_GETROUTE netlink op Message-ID: <20220709122320.7ecc9621@kernel.org> In-Reply-To: References: <20220707093336.214658-1-equinox@diac24.net> <20220708202951.46d3454a@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Sat, 9 Jul 2022 14:45:00 +0200 David Lamparter wrote: > > > + err = ip6mr_rtm_valid_getroute_req(in_skb, nlh, tb, extack); > > > + if (err < 0) > > > + goto errout; > > > > Can we: > > > > return err; > > > > ? I don't know where the preference for jumping to the return statement > > came from, old compilers? someone's "gut feeling"? > > If I were forced to find a justification, I'd say having a central > sequence of exit helps avoiding mistakes when some other resource > acquisition is added later. Easy to add a cleanup call to an existing > cleanup block - easy to overlook a "return err;" that needs to be > changed to "goto errout;". That only works if the label's name is meaningless, if the label is named after what it points to you have to rename the label and all the jumps anyway. Can as well replace returns with a goto. > But I have absolutely no stake in this at all, I'll happily edit it to > whatever the consensus is. This is just what the IPv4 code looks like > after being adapted for IPv6. Ah, I looked around other getroute implementations but not specifically ipmr. I'd rather refactor ipmr.c as well than keep its strangeness. The fact that we jump to the error path which tries to free the skb without ever allocating the skb feels particularly off.