Il mar 19 nov 2024, 00:06 Peter Xu <pet...@redhat.com> ha scritto:
> On Mon, Nov 18, 2024 at 05:13:30PM -0500, Peter Xu wrote:
> > When used incorrectly, container_get() can silently create containers
> even
> > if the caller may not intend to do so. Add a rich document describing
> the
> > helper, as container_get() should only be used in path lookups.
> >
> > Add one object_dynamic_cast() check to make sure whatever objects the
> > helper walks will be a container object (including the one to be
> returned).
> > It is a programming error otherwise, hence assert that.
> >
> > It may make container_get() tiny slower than before, but the hope is the
> > change is neglictable, as object_class_dynamic_cast() has a fast path
> just
> > for similar leaf use case.
>
> Just a heads up: out of curiosity, I tried to see whether the fast path hit
> that I mentioned here (mostly, commit 793c96b54032 of Paolo's), and it
> didn't..
>
> It's fundamentally because all TypeImpl was allocated dynamically from
> heap, including its type->name.
Ah, that was supposed to be the difference between type_register() and
type_register_static().
Perhaps type->name could be allocated with g_intern_string()? And then if
object_dynamic_cast() is changed into a macro, with something like
#define qemu_cache_interned_string(s) \
(__builtin_constant_p(s) \
? ({ static const char *interned; \
interned = interned ?: g_intern_static_string(s); }) \
: g_intern_string(s))
as the third parameter. This allows object_dynamic_cast() to use a simple
pointer equality for type name comparison, and the same can be applied to
object_class_dynamic_cast().
Whatever we do, we should do it before Rust code starts using
object_dynamic_cast!
Paolo
