Re: [bug report] staging: qlge: Initialize devlink health dump framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux