Besides the non-triviality of the patch, how is this different from object_property_get_link? Perhaps we should just rename that one to object_property_get_obj, or add a function that is just a synonym.
> + */ > +Object *object_property_get_child(Object *obj, const char *name, > + struct Error **errp); > + > +/** > * object_property_set: > * @obj: the object > * @v: the visitor that will be used to write the property value. This > should > diff --git a/qom/object.c b/qom/object.c > index 2de6eaf..61e775b 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -297,12 +297,86 @@ static void object_property_del_all(Object *obj) > } > } > > +/* > + * To ensure correct format checking, > + * this function should be used only via REPORT_OBJECT_ERROR() macro. > + * > + * The first argument after 'obj' should be of type 'const char *'. > + * It is ignored, and replaced by the canonical path of 'obj'. > + */ > +static void report_object_error(Error **errp, const char *fmt, Object > *obj, ...) > + GCC_FMT_ATTR(2, 4); > +static void report_object_error(Error **errp, const char *fmt, Object > *obj, ...) > +{ > + gchar *path; > + va_list ap; > + > + if (errp != NULL) { > + path = object_get_canonical_path(obj); > + va_start(ap, obj); > + va_arg(ap, const char *); /* Ignore the dummy string. */ Why do you need it at all? > + error_set(errp, fmt, path, &ap); > + va_end(ap); > + g_free(path); > + } > +} > +#define REPORT_OBJECT_ERROR(errp, fmt, obj, ...) \ > + do { \ > + report_object_error(errp, fmt, obj, "", ## __VA_ARGS__); \ > + } while (0) > +#define CHILD_PROPERTY_TYPE_PREFIX "child<" > +#define CHILD_PROPERTY_TYPE_SUFFIX ">" > +#define LINK_PROPERTY_TYPE_PREFIX "link<" > +#define LINK_PROPERTY_TYPE_SUFFIX ">" > + > +static bool object_property_is_child(ObjectProperty *prop) > +{ > + return (strstart(prop->type, CHILD_PROPERTY_TYPE_PREFIX, NULL) != 0); > +} > + > +static bool object_property_is_link(ObjectProperty *prop) > +{ > + return (strstart(prop->type, LINK_PROPERTY_TYPE_PREFIX, NULL) != 0); > +} > + > +/* Go from LINK_PROPERTY_TYPE_PREFIX FOO LINK_PROPERTY_TYPE_SUFFIX to > FOO. */ > +static gchar *link_type_to_type(const gchar *type) > +{ > + return g_strndup(&type[sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1], > + strlen(type) > + - (sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1) > + - (sizeof(LINK_PROPERTY_TYPE_SUFFIX) - 1)); > +} > + > +static Object *object_property_extract_child(ObjectProperty *prop, > + bool *p_is_invalid_type) > +{ > + if (p_is_invalid_type != NULL) { > + *p_is_invalid_type = false; > + } > + if (object_property_is_child(prop)) { > + return (Object *)prop->opaque; > + } else if (object_property_is_link(prop)) { > + if (prop->opaque != NULL) { > + return *(Object **)prop->opaque; > + } else { > + return NULL; > + } > + } else { > + if (p_is_invalid_type != NULL) { > + *p_is_invalid_type = true; > + } > + return NULL; > + } > +} > + object_property_get_child should never be on a fast path. We should avoid as much as possible peeking in the opaque. Instead, you should just use a visitor like object_property_get_link does. Everything starting here should be a separate patch. But if you remove object_property_extract_child, I doubt it makes as much sense to refactor this. > static void object_property_del_child(Object *obj, Object *child, Error > **errp) > { > ObjectProperty *prop; > > QTAILQ_FOREACH(prop, &obj->properties, node) { > - if (!strstart(prop->type, "child<", NULL)) { > + if (!object_property_is_child(prop)) { > continue; > } > > @@ -799,6 +873,27 @@ Object *object_get_root(void) > return root; > } > > +Object *object_property_get_child(Object *obj, const char *name, > + struct Error **errp) { > + Object *result = NULL; > + ObjectProperty *prop = object_property_find(obj, name); > + bool is_invalid_type; > + > + if (prop == NULL) { > + REPORT_OBJECT_ERROR(errp, QERR_OBJECT_PROPERTY_NOT_FOUND, obj, > name); > + return NULL; > + } > + > + result = object_property_extract_child(prop, &is_invalid_type); > + if (is_invalid_type) { > + REPORT_OBJECT_ERROR(errp, QERR_OBJECT_PROPERTY_INVALID_TYPE, > + obj, name, "child"); > + return NULL; > + } > + > + return result; > +} > + > static void object_get_child_property(Object *obj, Visitor *v, void > *opaque, > const char *name, Error **errp) > { > @@ -829,7 +924,10 @@ void object_property_add_child(Object *obj, const > char *name, > */ > assert(!object_is_type(obj, type_interface)); > > - type = g_strdup_printf("child<%s>", > object_get_typename(OBJECT(child))); > + type = g_strdup_printf(CHILD_PROPERTY_TYPE_PREFIX > + "%s" > + CHILD_PROPERTY_TYPE_SUFFIX, > + object_get_typename(OBJECT(child))); > > object_property_add(obj, name, type, object_get_child_property, > NULL, object_finalize_child_property, child, > errp); > @@ -878,8 +976,7 @@ static void object_set_link_property(Object *obj, > Visitor *v, void *opaque, > if (strcmp(path, "") != 0) { > Object *target; > > - /* Go from link<FOO> to FOO. */ > - target_type = g_strndup(&type[5], strlen(type) - 6); > + target_type = link_type_to_type(type); > target = object_resolve_path_type(path, target_type, &ambiguous); > > if (ambiguous) { > @@ -907,7 +1004,9 @@ void object_property_add_link(Object *obj, const > char *name, > { > gchar *full_type; > > - full_type = g_strdup_printf("link<%s>", type); > + full_type = g_strdup_printf(LINK_PROPERTY_TYPE_PREFIX > + "%s" > + LINK_PROPERTY_TYPE_SUFFIX, type); > > object_property_add(obj, name, full_type, > object_get_link_property, > @@ -932,7 +1031,7 @@ gchar *object_get_canonical_path(Object *obj) > g_assert(obj->parent != NULL); > > QTAILQ_FOREACH(prop, &obj->parent->properties, node) { > - if (!strstart(prop->type, "child<", NULL)) { > + if (!object_property_is_child(prop)) { > continue; > } > > @@ -980,15 +1079,7 @@ static Object *object_resolve_abs_path(Object > *parent, > return NULL; > } > > - child = NULL; > - if (strstart(prop->type, "link<", NULL)) { > - Object **pchild = prop->opaque; > - if (*pchild) { > - child = *pchild; > - } > - } else if (strstart(prop->type, "child<", NULL)) { > - child = prop->opaque; > - } > + child = object_property_extract_child(prop, NULL); > > if (!child) { > return NULL; > @@ -1010,7 +1101,7 @@ static Object *object_resolve_partial_path(Object > *parent, > QTAILQ_FOREACH(prop, &parent->properties, node) { > Object *found; > > - if (!strstart(prop->type, "child<", NULL)) { > + if (!object_property_is_child(prop)) { > continue; > } > > > > Paolo