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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1C268CD4F54 for ; Thu, 28 May 2026 20:48:25 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7898110E5E5; Thu, 28 May 2026 20:48:24 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="jOyHkSwV"; dkim-atps=neutral Received: from sea.source.kernel.org (sea.source.kernel.org [172.234.252.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8C2A510E5E5; Thu, 28 May 2026 20:48:23 +0000 (UTC) Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) by sea.source.kernel.org (Postfix) with ESMTP id 5EAF643F33; Thu, 28 May 2026 20:48:23 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4330F1F000E9; Thu, 28 May 2026 20:48:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1780001303; bh=FpQud1RDgcM36CY4JFAfny+s7huVK65jhoq2kaa9jkQ=; h=Date:From:Subject:Cc:To:References:In-Reply-To; b=jOyHkSwVCO70aJ4Ov/94NwYVAy9n5PAM8hPSejYwgQIExxEYZNmEbWq7U0TWFEOdd K32gMY1WhKQA+DvPEHgopFQc3JP2M0ykroQ8dB0Wj86HuT08+iBhBvGvOy6koG2EiL DAlnC/FvXx6253dt5qhslZ/cqwceHP6v9WSIOucB4rLfmQRgw2v3HNmZ3+Sel8rOKu 4umK7DtSkJPOLDgbPSa2qf1l2CLC/senmm3oCzL/LnF3rc+Ia/ky6LhbxeiBBKDOiq XDkoqqzaDzTE9xot3M3lN5ra9OrYN06Od/8WTxSA2lvsltKIzq50/WZUHYTy14vgHf FBmncuzlJWGBA== Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 28 May 2026 22:48:18 +0200 Message-Id: From: "Danilo Krummrich" Subject: Re: [PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage Cc: "lyude@redhat.com" , "rongqianfeng@vivo.com" , "airlied@gmail.com" , "kees@kernel.org" , "simona@ffwll.ch" , "dri-devel@lists.freedesktop.org" , "Zhi Wang" , "nouveau@lists.freedesktop.org" , "airlied@redhat.com" , "maarten.lankhorst@linux.intel.com" , "zenghongling@kylinos.cn" , "tzimmermann@suse.de" , "linux-kernel@vger.kernel.org" , "mripard@kernel.org" To: "Timur Tabi" , "Hongling Zeng" References: <20260528192847.4077458-1-lyude@redhat.com> <4f4b7a4a89a253e515b009ccfa7ddfd21853b64d.camel@nvidia.com> In-Reply-To: <4f4b7a4a89a253e515b009ccfa7ddfd21853b64d.camel@nvidia.com> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Thu May 28, 2026 at 10:23 PM CEST, Timur Tabi wrote: > On Thu, 2026-05-28 at 21:44 +0200, Danilo Krummrich wrote: >>=20 >> @Timur: I do think cleaning this up is the right call in general though,= and I >> also don't think that the whole driver necessarily needs to be consisten= t on >> whether IS_ERR_OR_NULL() or IS_ERR() is used -- it depends on the contex= t >> (although I usually prefer not to mix up NULL and ERR semantics in the f= irst >> place). >>=20 >> It should however be consistent in terms of what functions can actually = return. >>=20 >> ret =3D foo(); >> if (IS_ERR_OR_NULL(ret)) >> return ret; >>=20 >> If foo() can never return NULL, the above is misleading, as it puts an >> obligation on the caller to somehow handle the NULL case and come up wit= h an >> actual error code for it. > > Sure, I get that. My point is that it's often not clear whether foo() ac= tually can never return > NULL. =20 > > It's been a while since I've dug through the RPC call chains in Nouveau, = so my memory is a little > hazy here. I do remember noticing that Nouveau frequently has situations= where foo() call bar1() > and bar2(), where bar1() can return NULL but bar2() can't. So the questi= on is not whether foo() can > return NULL, it's whether bar1() should not return NULL, or whether bar2(= ) should. If there are multiple, it has to be the superset of course. >> So, I think it is the right call to align that to what functions can act= ually >> return, but while doing this, the contract should be properly documented= , such >> that subsequent changes can be properly validated. > > "Properly documented" and "Nouveau" are not two things that go together. Unfortunately -- but the changes submitted by Hongling can add the document= ation for the places that are touched. @Hongling, can you consider this in a v2 please? Thanks, Danilo 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id BE0C2CD6E45 for ; Thu, 28 May 2026 20:48:26 +0000 (UTC) Received: from kara.freedesktop.org (unknown [131.252.210.166]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6404610E6DD; Thu, 28 May 2026 20:48:26 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="jOyHkSwV"; dkim-atps=neutral Received: from kara.freedesktop.org (localhost [127.0.0.1]) by kara.freedesktop.org (Postfix) with ESMTP id D891C46683; Thu, 28 May 2026 20:34:56 +0000 (UTC) ARC-Seal: i=1; cv=none; a=rsa-sha256; d=lists.freedesktop.org; s=20240201; t=1780000496; b=Ea7CxiA5a6hvzboAg4t7FV3SRLZZ8yrCidtoZMlf21ugLEu9vUd4E15UQq045qxG+Jwsv qh8hH+8NT6GdVfszjH8Gz7V9GxbrTT65fRKvzAygSPmZz4B8z9n18VBJwrTz8+wMgO3dZ04 A1R97ptWEE78JnkF1ha+F+zajnZ/dtZmGDu7tXxFsqEONI/032sb0YamhYG8Bx7B/qSn1DI 63BuBfVQYZ3ZscGLmvgo7gd6ZV+17ZTJ6o04aL50H3FoEUMn/rf1M/N6e1Sa15k3NZG557P p3cypnww/5GOnVH87tcHXj/t4nTpx7m7XLYI3yJjgOMsnv7LkUpsgmoBn8fQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=lists.freedesktop.org; s=20240201; t=1780000496; h=from : sender : reply-to : subject : date : message-id : to : cc : mime-version : content-type : content-transfer-encoding : content-id : content-description : resent-date : resent-from : resent-sender : resent-to : resent-cc : resent-message-id : in-reply-to : references : list-id : list-help : list-unsubscribe : list-subscribe : list-post : list-owner : list-archive; bh=FpQud1RDgcM36CY4JFAfny+s7huVK65jhoq2kaa9jkQ=; b=HUFxYGc2zywwLFoLq0Qp1igmi1OcA5V4ys3CzadQIu1Wauk3DFOXkPjfg+qguC2M/K2eV QMl4rwa7rd+S3Nz/+xNvp474v1jdHEECrMhrhmLvK1vc0aMTITHQK3hU8rp8pvtDKiup25P dRneKpNZ6foWhv1XYbmW0GfWWUhJxSMdYy7XCMZRpAiNhxaQK+DLOaXT/rOQY3UAuy4Ev36 +/FNXWn46F/WA+MN8kxZSZB1yT5WMXYETVuidud+vCpZ907yOUe/mD6ul3pzB9FbF6wtKY6 UC2x9Uf0iX/Pe3QqIgFGAtvb6UA7HdNkcIJesV2d024EAZjcOQLPdB5hajAQ== ARC-Authentication-Results: i=1; mail.freedesktop.org; dkim=pass header.d=kernel.org; arc=none (Message is not ARC signed); dmarc=pass (Used From Domain Record) header.from=kernel.org policy.dmarc=quarantine Authentication-Results: mail.freedesktop.org; dkim=pass header.d=kernel.org; arc=none (Message is not ARC signed); dmarc=pass (Used From Domain Record) header.from=kernel.org policy.dmarc=quarantine Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by kara.freedesktop.org (Postfix) with ESMTPS id 3BBE04667C for ; Thu, 28 May 2026 20:34:54 +0000 (UTC) Received: from sea.source.kernel.org (sea.source.kernel.org [172.234.252.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8C2A510E5E5; Thu, 28 May 2026 20:48:23 +0000 (UTC) Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) by sea.source.kernel.org (Postfix) with ESMTP id 5EAF643F33; Thu, 28 May 2026 20:48:23 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4330F1F000E9; Thu, 28 May 2026 20:48:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1780001303; bh=FpQud1RDgcM36CY4JFAfny+s7huVK65jhoq2kaa9jkQ=; h=Date:From:Subject:Cc:To:References:In-Reply-To; b=jOyHkSwVCO70aJ4Ov/94NwYVAy9n5PAM8hPSejYwgQIExxEYZNmEbWq7U0TWFEOdd K32gMY1WhKQA+DvPEHgopFQc3JP2M0ykroQ8dB0Wj86HuT08+iBhBvGvOy6koG2EiL DAlnC/FvXx6253dt5qhslZ/cqwceHP6v9WSIOucB4rLfmQRgw2v3HNmZ3+Sel8rOKu 4umK7DtSkJPOLDgbPSa2qf1l2CLC/senmm3oCzL/LnF3rc+Ia/ky6LhbxeiBBKDOiq XDkoqqzaDzTE9xot3M3lN5ra9OrYN06Od/8WTxSA2lvsltKIzq50/WZUHYTy14vgHf FBmncuzlJWGBA== Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 28 May 2026 22:48:18 +0200 Message-Id: From: "Danilo Krummrich" Subject: Re: [PATCH 0/5] Revert cleanups for IS_ERR_OR_NULL() usage To: "Timur Tabi" , "Hongling Zeng" References: <20260528192847.4077458-1-lyude@redhat.com> <4f4b7a4a89a253e515b009ccfa7ddfd21853b64d.camel@nvidia.com> In-Reply-To: <4f4b7a4a89a253e515b009ccfa7ddfd21853b64d.camel@nvidia.com> Message-ID-Hash: 4PCDVZDTVWL43VOLTLL2EOAKIPHIXFHO X-Message-ID-Hash: 4PCDVZDTVWL43VOLTLL2EOAKIPHIXFHO X-MailFrom: dakr@kernel.org X-Mailman-Rule-Hits: nonmember-moderation X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation CC: "rongqianfeng@vivo.com" , "kees@kernel.org" , "simona@ffwll.ch" , "dri-devel@lists.freedesktop.org" , Zhi Wang , "nouveau@lists.freedesktop.org" , "airlied@redhat.com" , "maarten.lankhorst@linux.intel.com" , "zenghongling@kylinos.cn" , "linux-kernel@vger.kernel.org" , "mripard@kernel.org" X-Mailman-Version: 3.3.8 Precedence: list List-Id: Nouveau development list Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu May 28, 2026 at 10:23 PM CEST, Timur Tabi wrote: > On Thu, 2026-05-28 at 21:44 +0200, Danilo Krummrich wrote: >>=20 >> @Timur: I do think cleaning this up is the right call in general though,= and I >> also don't think that the whole driver necessarily needs to be consisten= t on >> whether IS_ERR_OR_NULL() or IS_ERR() is used -- it depends on the contex= t >> (although I usually prefer not to mix up NULL and ERR semantics in the f= irst >> place). >>=20 >> It should however be consistent in terms of what functions can actually = return. >>=20 >> ret =3D foo(); >> if (IS_ERR_OR_NULL(ret)) >> return ret; >>=20 >> If foo() can never return NULL, the above is misleading, as it puts an >> obligation on the caller to somehow handle the NULL case and come up wit= h an >> actual error code for it. > > Sure, I get that. My point is that it's often not clear whether foo() ac= tually can never return > NULL. =20 > > It's been a while since I've dug through the RPC call chains in Nouveau, = so my memory is a little > hazy here. I do remember noticing that Nouveau frequently has situations= where foo() call bar1() > and bar2(), where bar1() can return NULL but bar2() can't. So the questi= on is not whether foo() can > return NULL, it's whether bar1() should not return NULL, or whether bar2(= ) should. If there are multiple, it has to be the superset of course. >> So, I think it is the right call to align that to what functions can act= ually >> return, but while doing this, the contract should be properly documented= , such >> that subsequent changes can be properly validated. > > "Properly documented" and "Nouveau" are not two things that go together. Unfortunately -- but the changes submitted by Hongling can add the document= ation for the places that are touched. @Hongling, can you consider this in a v2 please? Thanks, Danilo