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 X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CFE8BC432C3 for ; Sat, 16 Nov 2019 00:59:18 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 880BA206D5 for ; Sat, 16 Nov 2019 00:59:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 880BA206D5 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=zeniv.linux.org.uk Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 47FH103tVMzF7w7 for ; Sat, 16 Nov 2019 11:59:16 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=ftp.linux.org.uk (client-ip=195.92.253.2; helo=zeniv.linux.org.uk; envelope-from=viro@ftp.linux.org.uk; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=zeniv.linux.org.uk Received: from ZenIV.linux.org.uk (zeniv.linux.org.uk [195.92.253.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 47FGZG61bQzF7nJ for ; Sat, 16 Nov 2019 11:39:32 +1100 (AEDT) Received: from viro by ZenIV.linux.org.uk with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1iVm5G-00041D-QH; Sat, 16 Nov 2019 00:37:02 +0000 Date: Sat, 16 Nov 2019 00:37:02 +0000 From: Al Viro To: Aleksa Sarai Subject: Re: [PATCH v16 02/12] namei: allow nd_jump_link() to produce errors Message-ID: <20191116003702.GX26530@ZenIV.linux.org.uk> References: <20191116002802.6663-1-cyphar@cyphar.com> <20191116002802.6663-3-cyphar@cyphar.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191116002802.6663-3-cyphar@cyphar.com> User-Agent: Mutt/1.12.1 (2019-06-15) X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Song Liu , linux-ia64@vger.kernel.org, linux-sh@vger.kernel.org, Peter Zijlstra , Rasmus Villemoes , Alexei Starovoitov , linux-kernel@vger.kernel.org, David Howells , linux-kselftest@vger.kernel.org, sparclinux@vger.kernel.org, containers@lists.linux-foundation.org, linux-api@vger.kernel.org, Shuah Khan , linux-arch@vger.kernel.org, linux-s390@vger.kernel.org, Tycho Andersen , Daniel Borkmann , Jiri Olsa , Alexander Shishkin , Ingo Molnar , linux-arm-kernel@lists.infradead.org, Yonghong Song , linux-mips@vger.kernel.org, Andrii Nakryiko , bpf@vger.kernel.org, linux-xtensa@linux-xtensa.org, Kees Cook , Arnd Bergmann , Jann Horn , linuxppc-dev@lists.ozlabs.org, dev@opencontainers.org, linux-m68k@lists.linux-m68k.org, Andy Lutomirski , Shuah Khan , Namhyung Kim , David Drysdale , Christian Brauner , "J. Bruce Fields" , libc-alpha@sourceware.org, Aleksa Sarai , linux-parisc@vger.kernel.org, netdev@vger.kernel.org, Chanho Min , Jeff Layton , Oleg Nesterov , Eric Biederman , linux-alpha@vger.kernel.org, linux-fsdevel@vger.kernel.org, Andrew Morton , Linus Torvalds , Martin KaFai Lau Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Sat, Nov 16, 2019 at 11:27:52AM +1100, Aleksa Sarai wrote: > + error = nd_jump_link(&path); > + if (error) > + path_put(&path); > + error = nd_jump_link(&ns_path); > + if (error) > + path_put(&ns_path); > + error = nd_jump_link(&path); > + if (error) > + path_put(&path); 3 calls. Exact same boilerplate in each to handle a failure case. Which spells "wrong calling conventions"; it's absolutely clear that we want that path_put() inside nd_jump_link(). The rule should be this: reference that used to be held in *path is consumed in any case. On success it goes into nd->path, on error it's just dropped, but in any case, the caller has the same refcounting environment to deal with. If you need the same boilerplate cleanup on failure again and again, the calling conventions are wrong and need to be fixed. And I'm not sure that int is the right return type here, to be honest. void * might be better - return ERR_PTR() or NULL, so that the value could be used as return value of ->get_link() that calls that thing. 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 X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7EF3DC432C3 for ; Sat, 16 Nov 2019 00:38:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5DB1620748 for ; Sat, 16 Nov 2019 00:38:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727409AbfKPAiK (ORCPT ); Fri, 15 Nov 2019 19:38:10 -0500 Received: from zeniv.linux.org.uk ([195.92.253.2]:43390 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727151AbfKPAiJ (ORCPT ); Fri, 15 Nov 2019 19:38:09 -0500 Received: from viro by ZenIV.linux.org.uk with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1iVm5G-00041D-QH; Sat, 16 Nov 2019 00:37:02 +0000 Date: Sat, 16 Nov 2019 00:37:02 +0000 From: Al Viro To: Aleksa Sarai Cc: Jeff Layton , "J. Bruce Fields" , Arnd Bergmann , David Howells , Shuah Khan , Shuah Khan , Ingo Molnar , Peter Zijlstra , Alexei Starovoitov , Daniel Borkmann , Martin KaFai Lau , Song Liu , Yonghong Song , Andrii Nakryiko , Eric Biederman , Andy Lutomirski , Andrew Morton , Kees Cook , Jann Horn , Tycho Andersen , David Drysdale , Chanho Min , Oleg Nesterov , Rasmus Villemoes , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Christian Brauner , Aleksa Sarai , Linus Torvalds , dev@opencontainers.org, containers@lists.linux-foundation.org, bpf@vger.kernel.org, netdev@vger.kernel.org, linux-alpha@vger.kernel.org, linux-api@vger.kernel.org, libc-alpha@sourceware.org, linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-fsdevel@vger.kernel.org, linux-ia64@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-m68k@lists.linux-m68k.org, linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org, linux-sh@vger.kernel.org, linux-xtensa@linux-xtensa.org, sparclinux@vger.kernel.org Subject: Re: [PATCH v16 02/12] namei: allow nd_jump_link() to produce errors Message-ID: <20191116003702.GX26530@ZenIV.linux.org.uk> References: <20191116002802.6663-1-cyphar@cyphar.com> <20191116002802.6663-3-cyphar@cyphar.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191116002802.6663-3-cyphar@cyphar.com> User-Agent: Mutt/1.12.1 (2019-06-15) Sender: linux-m68k-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-m68k@vger.kernel.org Message-ID: <20191116003702.McgxEvzNl1Bb8PU-e59ecn0wvaGQxeTmDJ-Z8BLSeJM@z> On Sat, Nov 16, 2019 at 11:27:52AM +1100, Aleksa Sarai wrote: > + error = nd_jump_link(&path); > + if (error) > + path_put(&path); > + error = nd_jump_link(&ns_path); > + if (error) > + path_put(&ns_path); > + error = nd_jump_link(&path); > + if (error) > + path_put(&path); 3 calls. Exact same boilerplate in each to handle a failure case. Which spells "wrong calling conventions"; it's absolutely clear that we want that path_put() inside nd_jump_link(). The rule should be this: reference that used to be held in *path is consumed in any case. On success it goes into nd->path, on error it's just dropped, but in any case, the caller has the same refcounting environment to deal with. If you need the same boilerplate cleanup on failure again and again, the calling conventions are wrong and need to be fixed. And I'm not sure that int is the right return type here, to be honest. void * might be better - return ERR_PTR() or NULL, so that the value could be used as return value of ->get_link() that calls that thing.