linux-kernel - Re: [PATCH v9 3/3] SPI: Add virtio SPI driver

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 for Android: free password hash cracker in your pocket

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

Message-ID: <aLgYLS6Lr5O2cIhK@smile.fi.intel.com>
Date: Wed, 3 Sep 2025 13:27:57 +0300
From: Andy Shevchenko <andriy.shevchenko@...el.com>
To: Haixu Cui <quic_haixcui@...cinc.com>
Cc: harald.mommer@....qualcomm.com, quic_msavaliy@...cinc.com,
	broonie@...nel.org, virtio-dev@...ts.linux.dev,
	viresh.kumar@...aro.org, linux-spi@...r.kernel.org,
	linux-kernel@...r.kernel.org, hdanton@...a.com,
	qiang4.zhang@...ux.intel.com, alex.bennee@...aro.org,
	quic_ztu@...cinc.com, virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH v9 3/3] SPI: Add virtio SPI driver

On Wed, Sep 03, 2025 at 05:04:46PM +0800, Haixu Cui wrote:
> On 9/1/2025 8:07 PM, Andy Shevchenko wrote:
> > On Thu, Aug 28, 2025 at 05:34:51PM +0800, Haixu Cui wrote:
> > > This is the virtio SPI Linux kernel driver.

...

> > > +#include <linux/completion.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/io.h>
> > > +#include <linux/module.h>
> > > +#include <linux/spi/spi.h>
> > > +#include <linux/stddef.h>
> > 
> > A lot of headers are still missing. See below.
> 
> This driver compiles successfully, and I believe all required definitions
> are resolved through indirect inclusion. For example, since I included
> virtio.h, there is no need to explicitly include device.h, scatterlist.h or
> types.h.
> 
> I avoided redundant #includes to keep the code clean and minimal.
> 
> If there are any essential headers I’ve overlooked, please feel free to
> highlight them—I’ll gladly include them in the next revision.

The rationale is described on https://include-what-you-use.org/.

...

> I plan to update the code as follows:
> 
> struct virtio_spi_req *spi_req __free(kfree) = NULL;
> spi_req = kzalloc(sizeof(*spi_req), GFP_KERNEL);
> if(!spi_req)
>     return -ENOMEM;
> 
> This follows the pattern used in
> virtio_net(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/virtio_net.c?h=v6.17-rc4#n3746)
> 
> I'd like to check if this style is acceptable here, thanks.

The style is fine. The potential issue (not now and probably never) is that the
scope of the variable in this case is different which might lead to unexpected
side-effects. That said, You can go with it.

-- 
With Best Regards,
Andy Shevchenko