Hello Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32868
to review the following change.
Change subject: device_tree: Make FDT property data non-const ......................................................................
device_tree: Make FDT property data non-const
FDT property data should not be const -- sometimes we need to update it, for example when fixing up phandles in an overlay. On the other hand it's occasionally desirable to put a string constant in there without having to strdup() it all the time... let's just live with the tiny implicit assumption that the data we'd want to modify (phandle references, mostly) will never be added from string constants, and put a cast in dt_add_string_prop().
Change-Id: Ifac103fcff0520cc427ab9a2aa141c65e12507ac Signed-off-by: Julius Werner jwerner@chromium.org --- M src/include/device_tree.h M src/lib/device_tree.c 2 files changed, 4 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/32868/1
diff --git a/src/include/device_tree.h b/src/include/device_tree.h index e3723c8..d9d9613 100644 --- a/src/include/device_tree.h +++ b/src/include/device_tree.h @@ -52,7 +52,7 @@ struct fdt_property { const char *name; - const void *data; + void *data; uint32_t size; };
@@ -165,7 +165,7 @@ void dt_delete_prop(struct device_tree_node *node, const char *name); // Add different kinds of properties to a node, or update existing ones. void dt_add_bin_prop(struct device_tree_node *node, const char *name, - const void *data, size_t size); + void *data, size_t size); void dt_add_string_prop(struct device_tree_node *node, const char *name, const char *str); void dt_add_u32_prop(struct device_tree_node *node, const char *name, u32 val); diff --git a/src/lib/device_tree.c b/src/lib/device_tree.c index 3c4bd24..a5021ca 100644 --- a/src/lib/device_tree.c +++ b/src/lib/device_tree.c @@ -883,7 +883,7 @@ * @param size The size of data in bytes. */ void dt_add_bin_prop(struct device_tree_node *node, const char *name, - const void *data, size_t size) + void *data, size_t size) { struct device_tree_property *prop;
@@ -955,7 +955,7 @@ void dt_add_string_prop(struct device_tree_node *node, const char *name, const char *str) { - dt_add_bin_prop(node, name, str, strlen(str) + 1); + dt_add_bin_prop(node, name, (char *)str, strlen(str) + 1); }
/*
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32868
to look at the new patch set (#4).
Change subject: device_tree: Make FDT property data non-const ......................................................................
device_tree: Make FDT property data non-const
FDT property data should not be const -- sometimes we need to update it, for example when fixing up phandles in an overlay. On the other hand it's occasionally desirable to put a string constant in there without having to strdup() it all the time... let's just live with the tiny implicit assumption that the data we'd want to modify (phandle references, mostly) will never be added from string constants, and put a cast in dt_add_string_prop().
Change-Id: Ifac103fcff0520cc427ab9a2aa141c65e12507ac Signed-off-by: Julius Werner jwerner@chromium.org --- M src/include/device_tree.h M src/lib/device_tree.c M src/soc/cavium/cn81xx/soc.c 3 files changed, 5 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/32868/4
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32868 )
Change subject: device_tree: Make FDT property data non-const ......................................................................
Patch Set 5: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32868 )
Change subject: device_tree: Make FDT property data non-const ......................................................................
device_tree: Make FDT property data non-const
FDT property data should not be const -- sometimes we need to update it, for example when fixing up phandles in an overlay. On the other hand it's occasionally desirable to put a string constant in there without having to strdup() it all the time... let's just live with the tiny implicit assumption that the data we'd want to modify (phandle references, mostly) will never be added from string constants, and put a cast in dt_add_string_prop().
Change-Id: Ifac103fcff0520cc427ab9a2aa141c65e12507ac Signed-off-by: Julius Werner jwerner@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/32868 Reviewed-by: Hung-Te Lin hungte@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/include/device_tree.h M src/lib/device_tree.c M src/soc/cavium/cn81xx/soc.c 3 files changed, 5 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Hung-Te Lin: Looks good to me, approved
diff --git a/src/include/device_tree.h b/src/include/device_tree.h index e3723c8..d9d9613 100644 --- a/src/include/device_tree.h +++ b/src/include/device_tree.h @@ -52,7 +52,7 @@ struct fdt_property { const char *name; - const void *data; + void *data; uint32_t size; };
@@ -165,7 +165,7 @@ void dt_delete_prop(struct device_tree_node *node, const char *name); // Add different kinds of properties to a node, or update existing ones. void dt_add_bin_prop(struct device_tree_node *node, const char *name, - const void *data, size_t size); + void *data, size_t size); void dt_add_string_prop(struct device_tree_node *node, const char *name, const char *str); void dt_add_u32_prop(struct device_tree_node *node, const char *name, u32 val); diff --git a/src/lib/device_tree.c b/src/lib/device_tree.c index 3c4bd24..a5021ca 100644 --- a/src/lib/device_tree.c +++ b/src/lib/device_tree.c @@ -883,7 +883,7 @@ * @param size The size of data in bytes. */ void dt_add_bin_prop(struct device_tree_node *node, const char *name, - const void *data, size_t size) + void *data, size_t size) { struct device_tree_property *prop;
@@ -955,7 +955,7 @@ void dt_add_string_prop(struct device_tree_node *node, const char *name, const char *str) { - dt_add_bin_prop(node, name, str, strlen(str) + 1); + dt_add_bin_prop(node, name, (char *)str, strlen(str) + 1); }
/* diff --git a/src/soc/cavium/cn81xx/soc.c b/src/soc/cavium/cn81xx/soc.c index efe7d6d..f1e11d3 100644 --- a/src/soc/cavium/cn81xx/soc.c +++ b/src/soc/cavium/cn81xx/soc.c @@ -149,7 +149,7 @@ if (*localmac) return; if (used_mac < num_free_mac_addresses) { - const u64 genmac = next_free_mac_address + used_mac; + u64 genmac = next_free_mac_address + used_mac; dt_add_bin_prop(node, name, &genmac, 6); used_mac++; return;
-
Hung-Te Lin (Code Review) -
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)