Bill XIE has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37109 )
Change subject: util/sconfig: Fix illogical override rule for resource ......................................................................
util/sconfig: Fix illogical override rule for resource
The old logic only use type to identify resources, which makes a resource in override tree overriding the first resource with the same type (but possibly different index) in base tree, and resources with same type (but again different index) in override tree overriding each other.
Resources had better be identified with both their type and index.
Change-Id: I7cd88905a8d6d1c7c6c03833835df2fba83047ea Signed-off-by: Bill XIE persmule@hardenedlinux.org --- M util/sconfig/main.c 1 file changed, 14 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/37109/1
diff --git a/util/sconfig/main.c b/util/sconfig/main.c index 5c23333..3b60e2a 100644 --- a/util/sconfig/main.c +++ b/util/sconfig/main.c @@ -1073,6 +1073,16 @@ }
/* + * Match resource nodes from base and override tree to see if they are the same + * node. + */ +static int res_match(struct resource *a, struct resource *b) +{ + return ((a->type == b->type) && + (a->index == b->index)); +} + +/* * Walk through the override subtree in breadth-first manner starting at node to * see if chip_instance pointer of the node is same as chip_instance pointer of * override parent that is passed into the function. If yes, then update the @@ -1104,8 +1114,7 @@ struct resource *base_res = dev->res;
while (base_res) { - if (base_res->type == res->type) { - base_res->index = res->index; + if (res_match(base_res, res)) { base_res->base = res->base; return; } @@ -1195,9 +1204,9 @@ * | | | * | res | Each resource that is present in override | * | | device is copied over to base device: | - * | | 1. If resource of same type is present in | - * | | base device, then index and base of the | - * | | resource is copied. | + * | | 1. If resource of same type and index is | + * | | present in base device, then base of | + * | | the resource is copied. | * | | 2. If not, then a new resource is allocated| * | | under the base device using type, index | * | | and base from override res. |
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37109 )
Change subject: util/sconfig: Fix illogical override rule for resource ......................................................................
Patch Set 1: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37109 )
Change subject: util/sconfig: Fix illogical override rule for resource ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/37109/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37109/1//COMMIT_MSG@9 PS1, Line 9: use uses the
Hello Kyösti Mälkki, Paul Menzel, build bot (Jenkins), Nico Huber, Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37109
to look at the new patch set (#2).
Change subject: util/sconfig: Fix illogical override rule for resource ......................................................................
util/sconfig: Fix illogical override rule for resource
The old logic only uses type to identify resources, which makes a resource in override tree overriding the first resource with the same type (but possibly different index) in base tree, and resources with same type (but again different index) in override tree overriding each other.
Resources had better be identified with both their type and index.
Change-Id: I7cd88905a8d6d1c7c6c03833835df2fba83047ea Signed-off-by: Bill XIE persmule@hardenedlinux.org --- M util/sconfig/main.c 1 file changed, 14 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/37109/2
Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37109 )
Change subject: util/sconfig: Fix illogical override rule for resource ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37109/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37109/1//COMMIT_MSG@9 PS1, Line 9: use
uses the
Done
Hello Kyösti Mälkki, Paul Menzel, build bot (Jenkins), Nico Huber, Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37109
to look at the new patch set (#3).
Change subject: util/sconfig: Fix illogical override rule for resource ......................................................................
util/sconfig: Fix illogical override rule for resource
The old logic only uses the type to identify resources, which makes a resource in override tree overriding the first resource with the same type (but possibly different index) in base tree, and resources with same type (but again different index) in override tree overriding each other.
Resources had better be identified with both their type and index.
Change-Id: I7cd88905a8d6d1c7c6c03833835df2fba83047ea Signed-off-by: Bill XIE persmule@hardenedlinux.org --- M util/sconfig/main.c 1 file changed, 14 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/37109/3
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37109 )
Change subject: util/sconfig: Fix illogical override rule for resource ......................................................................
Patch Set 5: Code-Review+2
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37109 )
Change subject: util/sconfig: Fix illogical override rule for resource ......................................................................
util/sconfig: Fix illogical override rule for resource
The old logic only uses the type to identify resources, which makes a resource in override tree overriding the first resource with the same type (but possibly different index) in base tree, and resources with same type (but again different index) in override tree overriding each other.
Resources had better be identified with both their type and index.
Change-Id: I7cd88905a8d6d1c7c6c03833835df2fba83047ea Signed-off-by: Bill XIE persmule@hardenedlinux.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/37109 Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M util/sconfig/main.c 1 file changed, 14 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/util/sconfig/main.c b/util/sconfig/main.c index 5c23333..3b60e2a 100644 --- a/util/sconfig/main.c +++ b/util/sconfig/main.c @@ -1073,6 +1073,16 @@ }
/* + * Match resource nodes from base and override tree to see if they are the same + * node. + */ +static int res_match(struct resource *a, struct resource *b) +{ + return ((a->type == b->type) && + (a->index == b->index)); +} + +/* * Walk through the override subtree in breadth-first manner starting at node to * see if chip_instance pointer of the node is same as chip_instance pointer of * override parent that is passed into the function. If yes, then update the @@ -1104,8 +1114,7 @@ struct resource *base_res = dev->res;
while (base_res) { - if (base_res->type == res->type) { - base_res->index = res->index; + if (res_match(base_res, res)) { base_res->base = res->base; return; } @@ -1195,9 +1204,9 @@ * | | | * | res | Each resource that is present in override | * | | device is copied over to base device: | - * | | 1. If resource of same type is present in | - * | | base device, then index and base of the | - * | | resource is copied. | + * | | 1. If resource of same type and index is | + * | | present in base device, then base of | + * | | the resource is copied. | * | | 2. If not, then a new resource is allocated| * | | under the base device using type, index | * | | and base from override res. |
-
Bill XIE (Code Review) -
Nico Huber (Code Review)
-
Paul Menzel (Code Review)