On 03.07.2018 22:04, Boris Brezillon wrote: > On Tue, 3 Jul 2018 17:19:57 +0300 > Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > >> Hello Stefan Agner, >> >> The patch d7d9f8ec77fe: "mtd: rawnand: add NVIDIA Tegra NAND Flash >> controller driver" from Jun 24, 2018, leads to the following static >> checker warning: >> >> drivers/mtd/nand/raw/tegra_nand.c:476 tegra_nand_select_chip() >> warn: array off by one? 'nand->cs[die_nr]' >> >> drivers/mtd/nand/raw/tegra_nand.c >> 465 static void tegra_nand_select_chip(struct mtd_info *mtd, int die_nr) >> 466 { >> 467 struct nand_chip *chip = mtd_to_nand(mtd); >> 468 struct tegra_nand_chip *nand = to_tegra_chip(chip); >> 469 struct tegra_nand_controller *ctrl = to_tegra_ctrl(chip->controller); >> 470 >> 471 if (die_nr < 0 || die_nr > 1) { >> 472 ctrl->cur_cs = -1; >> 473 return; >> 474 } >> 475 >> 476 ctrl->cur_cs = nand->cs[die_nr]; >> 477 } >> >> The story is that nand->cs[] is a one element array. Some people use >> one element arrays like this as variable size arrays. It's better to >> use a zero size array, but I think that might be a GCC feature and not >> everyone knows you can do that. Smatch treats this one as unknown size >> because apparently it can't tie it back to the kmalloc(). >> >> But it really is a one element array and the condition is off by one. > > I don't see where it's off by one? With the above test, die_nr is > guaranteed to be 0 when you reach the > "ctrl->cur_cs = nand->cs[die_nr];" statement, right? Am I missing > something? > Yeah I had to look twice too. But die_nr can be 1 according to this code... It should be: if (die_nr < 0 || die_nr >= 1) { >> >> But really one element arrays are super weird. Why not just use a >> pointer? > > The controller supports more than 1 CS, and I guess the plan was to > extend the array when the driver is ready to support this use case. I > guess we could make ->cs a single integer instead of an array of size > 1 if that helps. Exactly, and using the array also aligns with other drivers. I prefer to fix the issue above and leave it an array. I'll send a patch. -- Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html