On Wed, Mar 08, 2017 at 04:22:01PM -0300, Eduardo Habkost wrote:
> On Wed, Mar 08, 2017 at 09:06:06PM +0200, Krzysztof Kozlowski wrote:
> > > > The object_property_set_str() takes data as pointer to const. If data
> > > > ends up as being non-const, then this is the mistake -
> > > > object_property_set_str().
> > >
> > > I don't see the mistake. The whole purpose of:
> > > qdev_prop_set_chr(dev, "some-field", v)
> > > is to end up doing this assignment internally:
> > > dev->some_field = v;
> > > and on most (or all?) cases dev->some_field is not a const
> > > pointer. The details are hidden behind the
> > > object_property_set_str() call.
> >
> > If that would be the case, the object_property_set_str() cannot take a
> > pinter to const. Not only because of the safety and logic but also C
> > will prohibit it without a case.
> >
> > const char *c = "foo bar";
> > char *v = c;
> >
> > /home/dev/qemu/qemu/qobject/qstring.c:67:15: error: initialization
> > discards ‘const’ qualifier from pointer target type
> > [-Werror=discarded-qualifiers]
> > char *v = c;
> > ^
>
> The 'value' parameter to object_property_set_str() is const, and
> that's correct. But the set_chr() setter will take care of the
> 'dev->some_field = value' part.
In current implementation (v2.8.0-2132-gb64842dee42d) the
only thing qdev_prop_set_chr() does is to call
object_property_set_str() for value->label.
So the flow is:
qdev_prop_set_chr(char *value)
const char *l = value->label
object_property_set_str(const char *l)
dev->some_field = copy_of(l);
The only non-const part of the flow is the first call. All of others are
const.
Of course the implementation might change and maybe that is the
intention/plan - the qdev_prop_set_chr() should not commit to the caller
that it will not store the value itself.
Then I understand it.
However if there are no such plans, then in current implementation the
qdev_prop_set_chr() does not store any part of the 'value' itself but
only a copy of it through object_property_set_str(). Thus it can provide
this hint to the caller: I will not store the 'value' directly so I am
taking pointer to const.
Anyway, this is a trivial, boring and correctness-like change. :) Not worth
all the talks so I do not mind resending without this (and others which
were disapproved).
Best regards,
Krzysztof
