Re: [Qemu-trivial] [Qemu-devel] [PATCH] hw/sd/sdcard: Use the available enums

[Philippe Mathieu-Daudé]

On 5/6/19 10:55 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <address@hidden> writes:
> 
>> We already define SDCardModes/SDCardStates as enums. Declare
>> the mode/state as enums too, this make gdb debugging sessions
>> friendlier: instead of numbers, the mode/state name is displayed.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>> ---
>>  hw/sd/sd.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index aaab15f3868..a66b3d5b45e 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -96,8 +96,8 @@ struct SDState {
>>      BlockBackend *blk;
>>      bool spi;
>>  
>> -    uint32_t mode;    /* current card mode, one of SDCardModes */
>> -    int32_t state;    /* current card state, one of SDCardStates */
>> +    enum SDCardModes mode;
>> +    enum SDCardStates state;
>>      uint32_t vhs;
>>      bool wp_switch;
>>      unsigned long *wp_groups;
>> @@ -1640,7 +1640,7 @@ static int cmd_valid_while_locked(SDState *sd, 
>> SDRequest *req)
>>  
>>  int sd_do_command(SDState *sd, SDRequest *req,
>>                    uint8_t *response) {
>> -    int last_state;
>> +    enum SDCardStates last_state;
>>      sd_rsp_type_t rtype;
>>      int rsplen;
> 
> These guys are part of the migration state:
> 
>    static const VMStateDescription sd_vmstate = {
>        .name = "sd-card",
>        .version_id = 1,
>        .minimum_version_id = 1,
>        .pre_load = sd_vmstate_pre_load,
>        .fields = (VMStateField[]) {
>            VMSTATE_UINT32(mode, SDState),
>            VMSTATE_INT32(state, SDState),
>    [...]

Oh good point, I missed that.

> Juan, David, are VMSTATE_UINT32() and VMSTATE_INT32() safe to use with
> enums?

I'll wait Juan/Dave to enlighten us, else I can use anonymous union to
ease my debugging sessions:

-- >8 --
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
@@ -96,7 +96,10 @@ struct SDState {
     BlockBackend *blk;
     bool spi;

-    uint32_t mode;    /* current card mode, one of SDCardModes */
+    union {
+        uint32_t migratable_mode;
+        enum SDCardModes mode;
+    };
     int32_t state;    /* current card state, one of SDCardStates */
     uint32_t vhs;
     bool wp_switch;
@@ -659,7 +662,7 @@ static const VMStateDescription sd_vmstate = {
     .minimum_version_id = 1,
     .pre_load = sd_vmstate_pre_load,
     .fields = (VMStateField[]) {
-        VMSTATE_UINT32(mode, SDState),
+        VMSTATE_UINT32(migratable_mode, SDState),
         VMSTATE_INT32(state, SDState),
         VMSTATE_UINT8_ARRAY(cid, SDState, 16),
         VMSTATE_UINT8_ARRAY(csd, SDState, 16),
---

Thanks for the review!

Phil.