Hi Dan,

On Wed, Feb 03, 2021 at 12:42:50PM +0300, Dan Carpenter wrote:
Hello Coiby Xu,

The patch 953b94009377: "staging: qlge: Initialize devlink health
dump framework" from Jan 23, 2021, leads to the following static
checker warning:

        drivers/staging/qlge/qlge_devlink.c:163 qlge_health_create_reporters()
        error: uninitialized symbol 'reporter'.

drivers/staging/qlge/qlge_devlink.c
  151  void qlge_health_create_reporters(struct qlge_adapter *priv)
  152  {
  153          struct devlink_health_reporter *reporter;
  154          struct devlink *devlink;
  155
  156          devlink = priv_to_devlink(priv);
  157          priv->reporter =
  158                  devlink_health_reporter_create(devlink, 
&qlge_reporter_ops,
  159                                                 0, priv);
  160          if (IS_ERR(priv->reporter))
  161                  netdev_warn(priv->ndev,
  162                              "Failed to create reporter, err = %ld\n",
  163                              PTR_ERR(reporter));

Obviously the static checker is correct because we initialized
"priv->reporter" instead of "reporter".

It's not clear to me how "reporter" is used.  Presumably this should be:

        reporter = devlink_health_reporter_create(devlink, &qlge_reporter_ops,
                                                  0, priv);
        if (IS_ERR(reporter)) {
                netdev_warn(priv->ndev,
                            "Failed to create reporter, err = %ld\n",
                            PTR_ERR(reporter));
                return;
        }
        priv->reporter = reporter;


Thank you for finding this issue! "struct devlink_health_reporter
*reporter" is not needed since priv->reporter is used instead which
I think simplifies the code.

But I can't actually find where "priv->reporter" is checked against
NULL.  There should be some NULL checks, right?


There is no need to do NULL check since devlink_health_reporter_create
has done the job for us,

// net/core/devlink.c
__devlink_health_reporter_create(struct devlink *devlink,
                                 const struct devlink_health_reporter_ops *ops,
                                 u64 graceful_period, void *priv)
{
        reporter = kzalloc(sizeof(*reporter), GFP_KERNEL);
        if (!reporter)
                return ERR_PTR(-ENOMEM);

}

  164  }

regards,
dan carpenter

--
Best regards,
Coiby
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to