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