Hi Sascha,
Thanks for the review!
I've got one more question (below).
Am Freitag, den 25.05.2018, 09:11 +0200 schrieb Sascha Hauer:
> HI Florian,
>
> looks good generally. Some small comments below.
>
> On Thu, May 24, 2018 at 02:48:47PM +0000, Baeuerle, Florian wrote:
> > +static int of_fixup_property_set(struct device_node *root, void *context)
> > +{
> > + const struct of_fixup_property_data *fixup = context;
> > + int ret = do_of_property_set_now(root, fixup->path, fixup->propname,
> > + fixup->data, fixup->len);
> > +
> > + free(fixup->path);
> > + free(fixup->propname);
> > + free(fixup->data);
>
> Don't free here. A fixup can be called multiple times.
It looks like I should not free at all (of_fixup_status neither does).
Can you confirm that?
>
> > +
> > + return ret;
> > +}
> > +
> > +static int of_fixup_property_delete(struct device_node *root, void *context)
> > +{
> > + const struct of_fixup_property_data *fixup = context;
> > + int ret = do_of_property_delete_now(root, fixup->path, fixup->propname);
> > +
> > + free(fixup->path);
> > + free(fixup->propname);
>
> ditto.
>
> > +
> > + return ret;
> > +}
> > +
> > +static int do_of_property_set_fixup(const char *path, const char *propname,
> > + void *data, int len)
> > +{
> > + struct of_fixup_property_data *fixup;
> > + int ret = -ENOMEM;
> > +
> > + fixup = xzalloc(sizeof(*fixup));
> > + if (!fixup)
> > + goto out_fixup;
>
> The 'x' versions of the alloc functions always return successfully, no
> need to check.
>
> > +static int do_of_property_set_now(struct device_node *root, const char *path,
> > + const char *propname, void *data, int len)
> > +{
> > + struct device_node *node = of_find_node_by_path_or_alias(root, path);
> > + struct property *pp = NULL;
>
> No need to initialize.
>
> > +
> > + if (!node) {
> > + printf("Cannot find nodepath %s\n", path);
> > + return -ENOENT;
> > + }
> > +
> > + pp = of_find_property(node, propname, NULL);
> > +
> > + if (pp) {
> > + free(pp->value);
> > + pp->value_const = NULL;
> > +
> > + /* limit property data to the actual size */
> > + if (len)
> > + pp->value = xrealloc(data, len);
> > + else
> > + pp->value = NULL;
> > +
> > + pp->length = len;
> > + } else {
> > + pp = of_new_property(node, propname, data, len);
> > + if (!pp) {
> > + printf("Cannot create property %s\n", propname);
> > + return -ENOMEM;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int do_of_property_delete_now(struct device_node *root, const char *path,
> > + const char *propname)
> > +{
> > + struct device_node *node = of_find_node_by_path_or_alias(root, path);
> > + struct property *pp = NULL;
>
> ditto.
>
> Sascha
>
-- Florian
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
