On Thu, Sep 20, 2018 at 02:30:03PM -0700, Rob Herring wrote:
> Since commit 7975f6422260 ("Fix widespread incorrect use of strneq(),
> replace with new strprefixeq()") simple-bus checks have been silently
> skipped. The problem was 'end - str' is one more than the string length
> and the strnlen in strprefixeq fails. This can't be fixed simply by
> subtracting one as it is possible to have multiple '\0' at the end of
> the property. Fix this by making the 'compatible' property string list
> check a dependency, and then we can assume the property is null
> terminated and we can just use streq() for comparisons.
>
> Add some tests so the problem doesn't happen again.
>
> Fixes: 7975f6422260 ("Fix widespread incorrect use of strneq(), replace with new strprefixeq()")
> Reported-by: Kumar Gala <kumar.gala@xxxxxxxxxx>
> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
Applied, thanks.
> ---
> v2:
> - Make compatible_is_string_list a dependency so we can assume null
> terminated strings and simplify the implementation.
>
> checks.c | 5 +++--
> tests/run_tests.sh | 4 ++++
> tests/unit-addr-simple-bus-compatible.dts | 18 ++++++++++++++++++
> tests/unit-addr-simple-bus-reg-mismatch.dts | 18 ++++++++++++++++++
> 4 files changed, 43 insertions(+), 2 deletions(-)
> create mode 100644 tests/unit-addr-simple-bus-compatible.dts
> create mode 100644 tests/unit-addr-simple-bus-reg-mismatch.dts
>
> diff --git a/checks.c b/checks.c
> index 9c9b0c328af6..1c398d5a2427 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -910,7 +910,7 @@ static bool node_is_compatible(struct node *node, const char *compat)
>
> for (str = prop->val.val, end = str + prop->val.len; str < end;
> str += strnlen(str, end - str) + 1) {
> - if (strprefixeq(str, end - str, compat))
> + if (streq(str, compat))
> return true;
> }
> return false;
> @@ -921,7 +921,8 @@ static void check_simple_bus_bridge(struct check *c, struct dt_info *dti, struct
> if (node_is_compatible(node, "simple-bus"))
> node->bus = &simple_bus;
> }
> -WARNING(simple_bus_bridge, check_simple_bus_bridge, NULL, &addr_size_cells);
> +WARNING(simple_bus_bridge, check_simple_bus_bridge, NULL,
> + &addr_size_cells, &compatible_is_string_list);
>
> static void check_simple_bus_reg(struct check *c, struct dt_info *dti, struct node *node)
> {
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index adc4daebd945..6756f3d9f7be 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -668,6 +668,10 @@ dtc_tests () {
> check_tests pci-bridge-bad1.dts pci_bridge
> check_tests pci-bridge-bad2.dts pci_bridge
>
> + check_tests unit-addr-simple-bus-reg-mismatch.dts simple_bus_reg
> + check_tests unit-addr-simple-bus-compatible.dts simple_bus_reg
> +
> +
> # Check warning options
> run_sh_test dtc-checkfails.sh address_cells_is_cell interrupt_cells_is_cell -n size_cells_is_cell -- -Wno_size_cells_is_cell -I dts -O dtb bad-ncells.dts
> run_sh_test dtc-fails.sh -n test-warn-output.test.dtb -I dts -O dtb bad-ncells.dts
> diff --git a/tests/unit-addr-simple-bus-compatible.dts b/tests/unit-addr-simple-bus-compatible.dts
> new file mode 100644
> index 000000000000..c8f9341c1217
> --- /dev/null
> +++ b/tests/unit-addr-simple-bus-compatible.dts
> @@ -0,0 +1,18 @@
> +/dts-v1/;
> +
> +/ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + bus@10000000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "foo-bus", "simple-bus";
> + ranges = <0x0 0x10000000 0x10000>;
> +
> + node@100 {
> + reg = <0x1000 1>;
> + };
> + };
> +
> +};
> diff --git a/tests/unit-addr-simple-bus-reg-mismatch.dts b/tests/unit-addr-simple-bus-reg-mismatch.dts
> new file mode 100644
> index 000000000000..28233777097b
> --- /dev/null
> +++ b/tests/unit-addr-simple-bus-reg-mismatch.dts
> @@ -0,0 +1,18 @@
> +/dts-v1/;
> +
> +/ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + bus@10000000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "simple-bus";
> + ranges = <0x0 0x10000000 0x10000>;
> +
> + node@100 {
> + reg = <0x1000 1>;
> + };
> + };
> +
> +};
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
