On Mon, Feb 06 2023, Cornelia Huck <cohuck@xxxxxxxxxx> wrote: > On Mon, Feb 06 2023, Eric Auger <eauger@xxxxxxxxxx> wrote: > >> Hi, >> >> On 2/3/23 21:40, Richard Henderson wrote: >>> On 2/3/23 03:44, Cornelia Huck wrote: >>>> +static void aarch64_cpu_get_mte(Object *obj, Visitor *v, const char >>>> *name, >>>> + void *opaque, Error **errp) >>>> +{ >>>> + ARMCPU *cpu = ARM_CPU(obj); >>>> + OnOffAuto mte = cpu->prop_mte; >>>> + >>>> + visit_type_OnOffAuto(v, name, &mte, errp); >>>> +} >>> >>> You don't need to copy to a local variable here. >>> >>>> + >>>> +static void aarch64_cpu_set_mte(Object *obj, Visitor *v, const char >>>> *name, >>>> + void *opaque, Error **errp) >>>> +{ >>>> + ARMCPU *cpu = ARM_CPU(obj); >>>> + >>>> + visit_type_OnOffAuto(v, name, &cpu->prop_mte, errp); >>>> +} >>> >>> ... which makes get and set functions identical. >>> No need for both. >> This looks like a common pattern though. virt_get_acpi/set_acpi in >> virt.c or pc_machine_get_vmport/set_vmport in i386/pc.c and many other >> places (microvm ...). Do those other callers also need some simplifications? > > Indeed, I'm pretty sure that I copied + adapted it from somewhere :) > > Should we clean up all instances in one go instead? (Probably on top of > this series, in order to minimize conflicts with other changes.) Any objections to going with the code above and just doing a general cleanup on top?