On Fri, Oct 31, 2014 at 04:06:00PM +0300, Dan Carpenter wrote:
> Hello Aya Mahfouz,
>
Hello Dan,
> The patch be4aa8157c98: "staging: media: lirc: lirc_zilog.c: replace
> custom print macros with dev_* and pr_*" from Oct 26, 2014, leads to
> the following static checker warning:
>
> drivers/staging/media/lirc/lirc_zilog.c:1340 close()
> error: we previously assumed 'ir' could be null (see line 1339)
>
> drivers/staging/media/lirc/lirc_zilog.c
> 1333 /* Close the IR device */
> 1334 static int close(struct inode *node, struct file *filep)
> 1335 {
> 1336 /* find our IR struct */
> 1337 struct IR *ir = filep->private_data;
> 1338
> 1339 if (ir == NULL) {
> ^^^^^^^^^^
> 1340 dev_err(ir->l.dev, "close: no private_data attached to the file!\n");
> ^^^^^^^^^
>
> I suggest you just delete the error message. Can "ir" actually be NULL
> here anyway?
>
Since I'm a newbie and this is not my code, I prefer to use pr_err().
> 1341 return -ENODEV;
> 1342 }
> 1343
>
> drivers/staging/media/lirc/lirc_zilog.c:1636 ir_probe()
> error: potential null dereference 'ir'. (kzalloc returns null)
>
> drivers/staging/media/lirc/lirc_zilog.c
> 1614 dev_info(ir->l.dev, "IR unit on %s (i2c-%d) registered as lirc%d and ready\n",
> 1615 adap->name, adap->nr, ir->l.minor);
> 1616
> 1617 out_ok:
> 1618 if (rx != NULL)
> 1619 put_ir_rx(rx, true);
> 1620 if (tx != NULL)
> 1621 put_ir_tx(tx, true);
> 1622 put_ir_device(ir, true);
> 1623 dev_info(ir->l.dev, "probe of IR %s on %s (i2c-%d) done\n",
> 1624 tx_probe ? "Tx" : "Rx", adap->name, adap->nr);
> 1625 mutex_unlock(&ir_devices_lock);
> 1626 return 0;
> 1627
> 1628 out_put_xx:
> 1629 if (rx != NULL)
> 1630 put_ir_rx(rx, true);
> ^^^^^^^^^^^^^^^^^^^
> Btw, I think this is a double free? On some paths we call put_ir_rx()
> before the goto out_put_xx;.
>
I'll read about this and see how to fix it.
> 1631 if (tx != NULL)
> 1632 put_ir_tx(tx, true);
> 1633 out_put_ir:
> 1634 put_ir_device(ir, true);
> 1635 out_no_ir:
> 1636 dev_err(ir->l.dev, "%s: probing IR %s on %s (i2c-%d) failed with %d\n",
> ^^^^^^^^^
> Null dereference.
>
> 1637 __func__, tx_probe ? "Tx" : "Rx", adap->name, adap->nr,
> 1638 ret);
> 1639 mutex_unlock(&ir_devices_lock);
> 1640 return ret;
> 1641 }
>
In general, I can send a new patch to fix the aforementioned warnings.
Kindly let me know if you prefer that I send a second version of this
patch.
> regards,
> dan carpenter
Kind Regards,
Aya Saif El-yazal Mahfouz
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
