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?
>
> 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.
--
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
