On Fri, Aug 06 2021, David Hildenbrand <da...@redhat.com> wrote:
> On 06.08.21 11:42, Thomas Huth wrote:
>> On 05/08/2021 17.28, David Hildenbrand wrote:
>>> -static void qemu_s390_skeys_init(Object *obj)
>>> +static int qemu_s390_skeys_enabled(S390SKeysState *ss)
>>> {
>>> - QEMUS390SKeysState *skeys = QEMU_S390_SKEYS(obj);
>>> - MachineState *machine = MACHINE(qdev_get_machine());
>>> + QEMUS390SKeysState *skeys = QEMU_S390_SKEYS(ss);
>>>
>>> - skeys->key_count = machine->ram_size / TARGET_PAGE_SIZE;
>>> - skeys->keydata = g_malloc0(skeys->key_count);
>>> + /* Lockless check is sufficient. */
>>> + return !!skeys->keydata;
>>> }
>>>
>>> -static int qemu_s390_skeys_enabled(S390SKeysState *ss)
>>> +static int qemu_s390_skeys_enable(S390SKeysState *ss)
>>
>> Could you please call this qemu_s390_skeys_activate() so that it's not so
>> easily confused with the ..._enabled() function?
>
> Well, you enable it and you check if it's enabled ... so using different
> terminology is more confusing, no?
>
> I could rename _enabled to is_enabled to make the difference easiert to
> sport here.
is_enabled certainly is better than just enabled; otherwise, the two
function names are just way too similar.
