mtd: rawnand: Replace one-element array with flexible-array member

There is a regular need in the kernel to provide a way to declare having a dynamically sized set of trailing elements in a structure. Kernel code should always use “flexible array members”[1] for these cases. The older style of one-element or zero-length arrays should no longer be used[2].

Refactor the code according to the use of a flexible-array member instead of a one-element array. Also, make use of the struct_size() helper to calculate the size of the allocation for _nand_. In order to keep the code as maintainable as possible and to keep _cs_ as an array, add a new macro CS_N to aid in the allocation size calculation, in case there is a need for more Chip Select IDs in the future. In the meantime, the macro is set to 1. This also avoids having to use a magic number '1' as the last argument for struct_size().

drivers/mtd/nand/raw/tegra_nand.c +0 -0

drivers/mtd/nand/raw/tegra_nand.c +4 -1

+/* Number of Chip Selects. Currently, only one. */
+#define CS_N                   1
+
 struct tegra_nand_controller {
        struct nand_controller controller;
        struct device *dev;

@@ -183,7 +186,7 @@ struct tegra_nand_chip {
        u32 config;
        u32 config_ecc;
        u32 bch_config;
-       int cs[1];
+       int cs[];
 };

Quoted 4 lines · Gustavo A. R. Silva

There is a regular need in the kernel to provide a way to declare having a dynamically sized set of trailing elements in a structure. Kernel code should always use “flexible array members”[1] for these cases. The older style of one-element or zero-length arrays should no longer be used[2].

But this is not such a case, right? Isn't this a true fixed-size array? It sounds like you're just changing it because it pattern-matched on "array of length 1 at the end of a struct".

Quoted 9 lines

Refactor the code according to the use of a flexible-array member instead of a one-element array. Also, make use of the struct_size() helper to calculate the size of the allocation for _nand_. In order to keep the code as maintainable as possible and to keep _cs_ as an array, add a new macro CS_N to aid in the allocation size calculation, in case there is a need for more Chip Select IDs in the future. In the meantime, the macro is set to 1. This also avoids having to use a magic number '1' as the last argument for struct_size().

[...]

Quoted 1 line

drivers/mtd/nand/raw/tegra_nand.c +0 -0

[...]

Quoted 13 lines

drivers/mtd/nand/raw/tegra_nand.c +4 -1

+/* Number of Chip Selects. Currently, only one. */
+#define CS_N                   1
+
 struct tegra_nand_controller {
        struct nand_controller controller;
        struct device *dev;

@@ -183,7 +186,7 @@ struct tegra_nand_chip {
        u32 config;
        u32 config_ecc;
        u32 bch_config;
-       int cs[1];
+       int cs[];
 };

[...]

Quoted 10 lines · Jann Horn

Quoted 4 lines · Gustavo A. R. Silva

There is a regular need in the kernel to provide a way to declare having a dynamically sized set of trailing elements in a structure. Kernel code should always use “flexible array members”[1] for these cases. The older style of one-element or zero-length arrays should no longer be used[2].

But this is not such a case, right? Isn't this a true fixed-size array? It sounds like you're just changing it because it pattern-matched on "array of length 1 at the end of a struct".

Yeah; I should have changed that 'dynamically' part of the text above a bit. However, as I commented in the text below, in the case that more CS IDs are needed (let's wait for the maintainers to comment on this...) in the future, this change makes the code more maintainable, as for the allocation part, the developer would only have to update the CS_N macro to the number of CS IDs that are needed.

Quoted 12 lines · Gustavo A. R. Silva

Quoted 10 lines · Jann Horn

Quoted 4 lines · Gustavo A. R. Silva

There is a regular need in the kernel to provide a way to declare having a dynamically sized set of trailing elements in a structure. Kernel code should always use “flexible array members”[1] for these cases. The older style of one-element or zero-length arrays should no longer be used[2].

But this is not such a case, right? Isn't this a true fixed-size array? It sounds like you're just changing it because it pattern-matched on "array of length 1 at the end of a struct".

Quoted 6 lines

Yeah; I should have changed that 'dynamically' part of the text above a bit. However, as I commented in the text below, in the case that more CS IDs are needed (let's wait for the maintainers to comment on this...) in the future, this change makes the code more maintainable, as for the allocation part, the developer would only have to update the CS_N macro to the number of CS IDs that are needed.

But in that case, shouldn't you change it to "int cs[CS_N]" and get rid of the struct_size() stuff?

Hi Jann,

Jann Horn <[email protected]> wrote on Thu, 1 Oct 2020 00:32:24 +0200:

Quoted 21 lines

Quoted 18 lines · Gustavo A. R. Silva

Quoted 10 lines · Jann Horn

Quoted 4 lines · Gustavo A. R. Silva

There is a regular need in the kernel to provide a way to declare having a dynamically sized set of trailing elements in a structure. Kernel code should always use “flexible array members”[1] for these cases. The older style of one-element or zero-length arrays should no longer be used[2].

But this is not such a case, right? Isn't this a true fixed-size array? It sounds like you're just changing it because it pattern-matched on "array of length 1 at the end of a struct".

Yeah; I should have changed that 'dynamically' part of the text above a bit. However, as I commented in the text below, in the case that more CS IDs are needed (let's wait for the maintainers to comment on this...) in the future, this change makes the code more maintainable, as for the allocation part, the developer would only have to update the CS_N macro to the number of CS IDs that are needed.

Quoted 2 lines

But in that case, shouldn't you change it to "int cs[CS_N]" and get rid of the struct_size() stuff?

I do agree with Jann, I think it's best to consider this a fixed-size array for now. If we ever want to extend the number of supported CS, there is much more rework involved anyway.