Re: Input: cyapa - add gen5 trackpad device basic functions support

[Dan Carpenter <dan.carpenter@oracle.com>]

On Thu, Jan 22, 2015 at 01:50:59AM +0000, Dudley Du wrote:
> Hi Carpenter,
> 
> Thanks for the information.
> Could you indicate the tool and the command to generate this warning message?
> 

This a Smatch warning.

> In the code,
> 1) length = *gen5_pip->resp_len to get the expected response length,
> 2) then cyapa_empty_pip_output_data() try to polling the response data with the expect length,
> 3) at last, the length stored the real response length that it got in the polling function.
> 4) if the real response length is not 0, then assign the real response to replace the excpeted response length.

The error message is that we dereferenced gen5_pip->resp_len before we
checked whether it was NULL.  I believe you are saying that
cyapa_empty_pip_output_data() can modify "gen5_pip->resp_len" so we
need to do the check for NULL.

The problem is that I don't see where "gen5_pip->resp_len" gets changed
inside cyapa_empty_pip_output_data().  Smatch is supposed to do cross
function analysis and detect this but it doesn't see the modification
either.  I have been working on this code recently in Smatch so Smatch
may be buggy.

Can you help me out here so I can improve the tools?

According to Smatch "gen5_pip->resp_len" is set in two different
functions.

$ smdb where cyapa_gen5_cmd_states resp_len
drivers/input/mouse/cyapa_gen5.c | cyapa_gen5_initialize          | (struct cyapa_gen5_cmd_states)->resp_len | 0
drivers/input/mouse/cyapa_gen5.c | cyapa_i2c_pip_cmd_irq_sync     | (struct cyapa_gen5_cmd_states)->resp_len | 0,4096-2117777777777777777

Also I looked at the call tree to see if cyapa_empty_pip_output_data
calls cyapa_i2c_pip_cmd_irq_sync but it doesn't.

$ smdb call_tree cyapa_i2c_pip_cmd_irq_sync | grep cyapa_empty_pip_output_data

But, uh..  it's been years since I tried looking at the call_tree code
so I have no idea if it works...

regards,
dan carpenter