linux-kernel - RE: [PATCH v1 2/3] iommu/vt-d: Change dmar_ats

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DS0PR11MB63739B7AA12D735DBDC3ABC9DC8AA@DS0PR11MB6373.namprd11.prod.outlook.com>
Date: Fri, 9 May 2025 11:21:57 +0000
From: "Wang, Wei W" <wei.w.wang@...el.com>
To: "Liu, Yi L" <yi.l.liu@...el.com>, "dwmw2@...radead.org"
	<dwmw2@...radead.org>, "baolu.lu@...ux.intel.com" <baolu.lu@...ux.intel.com>,
	"Tian, Kevin" <kevin.tian@...el.com>, "jroedel@...e.de" <jroedel@...e.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"iommu@...ts.linux.dev" <iommu@...ts.linux.dev>
Subject: RE: [PATCH v1 2/3] iommu/vt-d: Change dmar_ats_supported() to return
 boolean

On Friday, May 9, 2025 5:27 PM, Liu, Yi L wrote:
/5/9 22:00, Wei Wang wrote:
> > According to "Function return values and names" in coding-style.rst,
> > the
> > dmar_ats_supported() function should return a boolean instead of an
> > integer. Also, rename "ret" to "supported" to be more straightforward.
> >
> 
> seems just a recommendation since this is just internal helper. The function
> was indeed not well written anyhow. :) not sure if Baolu wants take it or not.
> Taking it may make history tracking harder. Patch itself looks good to me.
> 
> Reviewed-by: Yi Liu <yi.l.liu@...el.com>

It seems mandatory to me from coding-style.rst:
"
Mixing up these two sorts of representations is a fertile source of
difficult-to-find bugs.  If the C language included a strong distinction
between integers and booleans then the compiler would find these mistakes
for us... but it doesn't.  To help prevent such bugs, always follow this
convention::

        If the name of a function is an action or an imperative command,
        the function should return an error-code integer.  If the name
        is a predicate, the function should return a "succeeded" boolean.
"

Another reason for making this change is that when we try to add more checks
inside this function (e.g., integrated_device_ats_supported() in patch 3), the added
new function needs to continue following the incorrect style. So, I thought, why
not change it from the foundation 😊