I can then leave only
+ if (!dev) {
+ ret = -ENODEV;
+ goto err;
+ }
+
but make it higher
How do you like this option?
пт, 16 мая 2025 г. в 19:20, Simon Glass <s...@chromium.org>:
> Hi,
>
> On Fri, 16 May 2025 at 14:51, <ant.v.morya...@gmail.com> wrote:
> >
> > From: Anton Moryakov <ant.v.morya...@gmail.com>
> >
> > The static analyzer (Svace) reported
> > After having been compared to a NULL value at spi-uclass.c:465,
> > pointer 'dev' is passed as 1st parameter in call to function
> 'dev_get_flags'
> > at spi-uclass.c:469, where it is dereferenced at device.h:240.
> >
> > Correct explained:
> > 1. Added dev && !device_active(dev) check before calling device_active()
> > 2. Added explicit if (!dev) check with ret = -ENODEV setting
> > 3. Protected logging in error block with if(dev) check
> >
> > Signed-off-by: Anton Moryakov <ant.v.morya...@gmail.com>
> > ---
> > drivers/spi/spi-uclass.c | 14 +++++++++++---
> > 1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
> > index d6049753740..52b79223f96 100644
> > --- a/drivers/spi/spi-uclass.c
> > +++ b/drivers/spi/spi-uclass.c
> > @@ -345,7 +345,7 @@ int spi_get_bus_and_cs(int busnum, int cs, struct
> udevice **busp,
> > return ret;
> > }
> >
> > - if (!device_active(dev)) {
> > + if (dev && !device_active(dev)) {
> > struct spi_slave *slave;
> >
> > ret = device_probe(dev);
> > @@ -355,6 +355,11 @@ int spi_get_bus_and_cs(int busnum, int cs, struct
> udevice **busp,
> > slave->dev = dev;
> > }
> >
> > + if (!dev) {
> > + ret = -ENODEV;
> > + goto err;
> > + }
> > +
> > slave = dev_get_parent_priv(dev);
> > bus_data = dev_get_uclass_priv(bus);
> >
> > @@ -373,8 +378,11 @@ int spi_get_bus_and_cs(int busnum, int cs, struct
> udevice **busp,
> > return 0;
> >
> > err:
> > - log_debug("%s: Error path, device '%s'\n", __func__, dev->name);
> > -
> > + if(dev)
> > + log_debug("%s: Error path, device '%s'\n", __func__,
> dev->name);
> > + else
> > + log_debug("%s: Error path, NULL device\n", __func__);
> > +
> > return ret;
> > }
> >
> > --
> > 2.30.2
> >
>
> You are not allowed to pass NULL as dev to driver model functions. So
> the bug here is in the SPI code.
>
> There is a code-size cost to checking arguments and we tend to be
> reluctant to pay it in U-Boot, where code size is often critical.
>
> We do have assert() but it is fairly intrusive and people seldom
> enable it. If you have a better way, please suggest it!
>
> Regards,
> Simon
>
