On Mon, Jan 30, 2017 at 09:07:15AM +0100, Uwe Kleine-König wrote: > Hello, > > On Mon, Jan 30, 2017 at 08:12:17AM +0100, Thierry Reding wrote: > > On Mon, Jan 30, 2017 at 10:33:07AM +0530, Shailendra Verma wrote: > > > of_device_get_match_data could return NULL, and so can cause > > > a NULL pointer dereference later. > > > > > > Signed-off-by: Shailendra Verma <shailendra.v@xxxxxxxxxxx> > > > --- > > > drivers/i2c/busses/i2c-tegra.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > This will never happen. Any match in the OF table that would cause the > > ->probe() to occur has a valid .data pointer associated with it. > > Theoretically you could (I think) bind that driver to a node with > > compatible = "tegra-i2c"; That's not a valid compatible string and I don't think this could end up anywhere that would make the driver bind. Even if it did I think it'd be good to crash rather than error out to make it very obvious that you've made a mistake that needs to be immediately fixed. If you error out it's much more likely that people won't notice. > Anyhow, even if today there was no possibility this could happen, that's > something that might easily be changed by a future change. So I doubt > "this will never happen" stays true for sure and being defensive is a > good idea. Let's revisit this again *if* this ever becomes a real issue. There's no use in adding dead code to the kernel to handle hypothetical use-cases. > And even a BUG would be better than a silent NULL pointer dereference. I've never encountered a NULL pointer dereference that was silent. =) Thierry
Attachment:
signature.asc
Description: PGP signature
