Re: [Qemu-devel] [PATCH v2] Revised: Add object_property_get_child().

[Alexander Barabash]

On 02/19/2012 07:14 PM, Andreas Färber wrote:
> Am 19.02.2012 17:04, schrieb Alexander Barabash:
> > ...
> >      Signed-off-by: Alexander Barabash<address@hidden>
> Please use git-send-email to submit your patches: The commit message is
> unnecessarily indented and the first line is duplicated. Instead of
> "Revised: " (which v2 already indicates) the subject should mention the
> topic, here "qom: ".
>
> http://wiki.qemu.org/Contribute/SubmitAPatch

Thanks for the advice.

> Your patch is doing too many things at once. Please split it up into a
> series of easily digestable and bisectable patches, e.g.:
> * ..._PREFIX/SUFFIX introduction and use in _add_child/link,
> * ..._is_child/_is_link introduction and use in
> _property_del_child/_get_canonical_path/_resolve_partial_path,
> * link_type_to_type() and use in _set_link_property,
> * REPORT_OBJECT_ERROR(),
> * object_property_get_child().

OK

> > +/* 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));
> Any reason not to use strlen()? I don't think this is a hot path, and
> repeated sizeof() - 1 does not read so nice. The alternative would be to
> hardcode the offsets.

I replaced "5" (i.e. the length "link<") with 
"sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1".
If we assume that "strlen("link<") is always optimized away,
then certainly using strlen is equivalent and looks better.

> Andreas
Alex