Received: by 2002:a05:6a10:17d3:0:0:0:0 with SMTP id hz19csp602593pxb; Fri, 16 Apr 2021 13:23:35 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwrIRxLopooi0lCzEWYK/1X3IhGowW1IbsJsIaokyU9NHAx58cuMCxVpsW/kArphI1H405S X-Received: by 2002:a17:90b:30c3:: with SMTP id hi3mr11045522pjb.27.1618604615311; Fri, 16 Apr 2021 13:23:35 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1618604615; cv=none; d=google.com; s=arc-20160816; b=DEVgK0KdSBlCijzsycqF8cBPoCvJqZfwGL1mAwcKLnjnUCZEOR+kjQlezp9cv338Q8 tRx6dV6ayV9MDVd1taXiy6BM7wd9KNhF/ph4jzIx+vEG5cQHRMEz+dQSt+wEa7i+csrQ q7vxUSq7CtaDjm+yQ7F2LKmuRPsgBRwXLu8OWn35wEQR20mWrBb9wy996aa2/F5AUSPv XoO19DE5Mngnl2g1czyqdsONdwEzB85mSQdtgWWsMTK+CRacpR14jGnCNpC5nPlS+znF 2nqwQgoFY0uFvGCQ6RF3D0e/I1dxnKR0Q4r5GPH7DifaF3ilIKnu8ZR38QvPhw6spy7I 8QjA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:user-agent:in-reply-to:content-disposition :mime-version:references:message-id:subject:cc:to:from:date; bh=+Rt9Vf6uIkFQbBWcPXbaTpACXzMaXiGxm01601kFwCM=; b=t+BsqSUKuId33nf7hA3OGapYZXTttdnQDcP27APK+Nn37wbf4dbvDYrf3MqXnW/ZSs pWVialMljRbgQdZqZ0zdWybf/y43LN0Z0VwCLWbujskZU6s/R/yxrExjMzkyUblJ6NIv Pi4GsXksOR+64SKqP5Wun+wClEzna2uPeGeTQZRIaoVGKQMLCdkiAVTlEnO5vCkfzuWt IlcpdG0S+AvXGhrr9a+dDlUhmic33FvWJ1LqjCgWWQS+5va/zr4NDTbTt3xJlwza2LR7 oStiVeWhDWEoMcPfmbjU3t6eQ2E1eOPP9+KO8l1qfgEUU4TlfrVXvdEXTZYy8Jl1SdjI AHKQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id i184si7995023pgd.441.2021.04.16.13.23.22; Fri, 16 Apr 2021 13:23:35 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244908AbhDPUXM (ORCPT + 99 others); Fri, 16 Apr 2021 16:23:12 -0400 Received: from wtarreau.pck.nerim.net ([62.212.114.60]:51740 "EHLO 1wt.eu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234654AbhDPUXM (ORCPT ); Fri, 16 Apr 2021 16:23:12 -0400 Received: (from willy@localhost) by pcw.home.local (8.15.2/8.15.2/Submit) id 13GKMFDa011534; Fri, 16 Apr 2021 22:22:15 +0200 Date: Fri, 16 Apr 2021 22:22:15 +0200 From: Willy Tarreau To: Miguel Ojeda Cc: Al Viro , Linus Torvalds , Peter Zijlstra , Miguel Ojeda , Greg Kroah-Hartman , rust-for-linux@vger.kernel.org, Linux Kbuild mailing list , "open list:DOCUMENTATION" , Linux Kernel Mailing List , Alex Gaynor , Geoffrey Thomas , Finn Behrens , Adam Bratschi-Kaye , Wedson Almeida Filho , Michael Ellerman Subject: Re: [PATCH 04/13] Kbuild: Rust support Message-ID: <20210416202215.GA11236@1wt.eu> References: <20210414184604.23473-1-ojeda@kernel.org> <20210414184604.23473-5-ojeda@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Apr 16, 2021 at 08:57:07PM +0200, Miguel Ojeda wrote: > On Fri, Apr 16, 2021 at 8:10 PM Al Viro wrote: > > > > How well would ? operator fit that pattern? _If_ it's just a syntax sugar > > along the lines of "if argument matches Err(_), return Err(_)", the types > > shouldn't be an issue, but that might need some fun with releasing resources, > > etc. If it's something more elaborate... details, please. > > Yes, it is just syntax sugar -- it doesn't introduce any power to the language. > > It was introduced because it is a very common pattern when using the > `Result` and `Option` enums. In fact, before it existed, it was just a > simple macro that you could also implement yourself. > > For instance, given `Foo` and `Bar` types that need RAII cleanup of > some kind (let's say `kill_foo()` and `kill_bar()`): > > fn foo() -> KernelResult { > if black_box() { > return Err(EINVAL); > } > > // something that gets you a `Foo` > let foo = ...; > > Ok(foo) > } > > fn bar() -> KernelResult { > let p = foo()?; > > // something that gets you a `Bar`, possibly using the `p` > let bar = ...; > > Ok(bar) > } > > This reduces to (full example at https://godbolt.org/z/hjTxd3oP1): > > bar: > push rbx > mov ebx, 1 > call qword ptr [rip + black_box@GOTPCREL] > test al, al > jne .LBB2_2 > call qword ptr [rip + kill_foo@GOTPCREL] > xor ebx, ebx > .LBB2_2: > mov eax, ebx > mov edx, -1234 > pop rbx > ret > > You can see `bar()` calls `black_box()`. If it failed, it returns the > EINVAL. Otherwise, it cleans up the `foo` automatically and returns > the successful `bar`. So it simply does the equivalent of: #define EINVAL -1234 struct result { int status; int error; }; extern bool black_box(); extern void kill_foo(); struct result bar() { return (struct error){ !!black_box(), EINVAL }; } struct result foo() { struct result res = bar(); if (res.status) goto leave; /* ... */ kill_foo(); // only for rust, C doesn't need it leave: return res; } So it simply returns a pair of values instead of a single one, which is nothing special but not much conventional in the kernel either given that ultimately syscalls will usually return a single value anyway. At some point some code will have to remerge them to provide a composite result and even take care of ambigous special cases like { true, 0 } which may or may not indicate an error, and avoiding to consider the unused error code on success. For example some code called from mmap() might have to deal with this this way: if (result.status == (void *)-1) return -result.error; else return result.status; But possibly a lot of code feeding this result struct would be tempted to do something like this to deal with NULL returns: result.status = foo_alloc(); if (!result.status) { result.error = -ENOMEM; return result; } And suddenly the error is lost, as a NULL is returned to the upper layers which does not correspond to an failure status. Conversely, with a unique result you'd do something like this: result = foo_alloc(); if (!result) return -ENOMEM; So it might possibly be safer to stick to the usually expected return values instead of introducing composite ones. I tend to agree that composite results can be better from new projects started from scratch when all the API follows this principle, but here there's quite a massive code base that was not designed along these lines and I easily see how this can become a source of trouble over time. Willy