Re: [PATCH 1/2] coresight: stm: Remove redundant NULL checks

[Mike Leach <mike.leach@linaro.org>]

On Thu, 12 Jun 2025 at 06:19, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
>
>
> On 11/06/25 8:14 PM, Leo Yan wrote:
> > container_of() cannot return NULL, so the checks for NULL pointers are
> > unnecessary and can be safely removed.
> >
> > As a result, this commit silences the following smatch warnings:
> >
> >   coresight-stm.c:345 stm_generic_link() warn: can 'drvdata' even be NULL?
> >   coresight-stm.c:356 stm_generic_unlink() warn: can 'drvdata' even be NULL?
> >   coresight-stm.c:387 stm_generic_set_options() warn: can 'drvdata' even be NULL?
> >   coresight-stm.c:422 stm_generic_packet() warn: can 'drvdata' even be NULL?
> >
> > Signed-off-by: Leo Yan <leo.yan@arm.com>
> > ---
> >  drivers/hwtracing/coresight/coresight-stm.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> > index e45c6c7204b4491e0f879bc7d5d445aa1d3118be..464b0c85c3f7d3519169d62a51e9f8c6281b5358 100644
> > --- a/drivers/hwtracing/coresight/coresight-stm.c
> > +++ b/drivers/hwtracing/coresight/coresight-stm.c
> > @@ -342,7 +342,7 @@ static int stm_generic_link(struct stm_data *stm_data,
> >  {
> >       struct stm_drvdata *drvdata = container_of(stm_data,
> >                                                  struct stm_drvdata, stm);
> > -     if (!drvdata || !drvdata->csdev)
> > +     if (!drvdata->csdev)
> >               return -EINVAL;
> >
I'd agree that the container_of() cannot return NULL, but what happens
if stm_data is NULL?
Perhaps the NULL check was on the wrong parameter?

Mike

> >       return coresight_enable_sysfs(drvdata->csdev);
> > @@ -353,7 +353,7 @@ static void stm_generic_unlink(struct stm_data *stm_data,
> >  {
> >       struct stm_drvdata *drvdata = container_of(stm_data,
> >                                                  struct stm_drvdata, stm);
> > -     if (!drvdata || !drvdata->csdev)
> > +     if (!drvdata->csdev)
> >               return;
> >
> >       coresight_disable_sysfs(drvdata->csdev);
> > @@ -384,7 +384,7 @@ static long stm_generic_set_options(struct stm_data *stm_data,
> >  {
> >       struct stm_drvdata *drvdata = container_of(stm_data,
> >                                                  struct stm_drvdata, stm);
> > -     if (!(drvdata && coresight_get_mode(drvdata->csdev)))
> > +     if (!coresight_get_mode(drvdata->csdev))
> >               return -EINVAL;
> >
> >       if (channel >= drvdata->numsp)
> > @@ -419,7 +419,7 @@ static ssize_t notrace stm_generic_packet(struct stm_data *stm_data,
> >                                                  struct stm_drvdata, stm);
> >       unsigned int stm_flags;
> >
> > -     if (!(drvdata && coresight_get_mode(drvdata->csdev)))
> > +     if (!coresight_get_mode(drvdata->csdev))
> >               return -EACCES;
> >
> >       if (channel >= drvdata->numsp)
> >
>
> Seems to be a sensible clean up.
>
> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
>


-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK