Oh crap, this is from 4 years ago. Never mind. I try to not send
emails for stuff that's over a couple years old but I didn't notice the
date here.
regards,
dan carpenter
On Tue, May 14, 2019 at 12:49:21PM +0300, Dan Carpenter wrote:
> Hello Karicheri, Muralidharan,
>
> The patch 84640e27f230: "net: netcp: Add Keystone NetCP core ethernet
> driver" from Jan 15, 2015, leads to the following static checker
> warning:
>
> drivers/net/ethernet/ti/netcp_core.c:1599 netcp_setup_navigator_resources()
> warn: passing zero to 'PTR_ERR'
>
> drivers/net/ethernet/ti/netcp_core.c
> 1583 static int netcp_setup_navigator_resources(struct net_device *ndev)
> 1584 {
> 1585 struct netcp_intf *netcp = netdev_priv(ndev);
> 1586 struct knav_queue_notify_config notify_cfg;
> 1587 struct knav_dma_cfg config;
> 1588 u32 last_fdq = 0;
> 1589 u8 name[16];
> 1590 int ret;
> 1591 int i;
> 1592
> 1593 /* Create Rx/Tx descriptor pools */
> 1594 snprintf(name, sizeof(name), "rx-pool-%s", ndev->name);
> 1595 netcp->rx_pool = knav_pool_create(name, netcp->rx_pool_size,
> 1596 netcp->rx_pool_region_id);
> 1597 if (IS_ERR_OR_NULL(netcp->rx_pool)) {
> 1598 dev_err(netcp->ndev_dev, "Couldn't create rx pool\n");
> 1599 ret = PTR_ERR(netcp->rx_pool);
> 1600 goto fail;
> 1601 }
> 1602
> 1603 snprintf(name, sizeof(name), "tx-pool-%s", ndev->name);
> 1604 netcp->tx_pool = knav_pool_create(name, netcp->tx_pool_size,
> 1605 netcp->tx_pool_region_id);
> 1606 if (IS_ERR_OR_NULL(netcp->tx_pool)) {
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> Assume this is NULL.
>
> When a function returns a mix of error pointers and NULL that means the
> the feature can be disabled. Like an error pointer is straight forward,
> it means that something is broken. But a NULL just means the feature is
> disabled so it's not an error but we also don't have a pointer to the
> feature because it doesn't exist.
>
> In this case, how can the driver function without memory pools? It
> doesn't make sense that the user should be able to disable it.
>
> 1607 dev_err(netcp->ndev_dev, "Couldn't create tx pool\n");
>
> If it's not an error, we shouldn't print an error message.
>
> 1608 ret = PTR_ERR(netcp->tx_pool);
>
> PTR_ERR(NULL) is zero/success.
>
> 1609 goto fail;
> 1610 }
> 1611
>
> This driver mentions ERR_PTR_OR_NULL() a lot but all the mentions I saw
> were wrong. :( I can't compile the driver myself or I would send a
> patch.
>
> regards,
> dan carpenter
