[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/3] hw/sd.c: add SD card save/load support

From: Mitsyanko Igor
Subject: Re: [Qemu-devel] [PATCH 2/3] hw/sd.c: add SD card save/load support
Date: Tue, 27 Dec 2011 17:54:57 +0400
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20111109 Thunderbird/3.1.16

On 12/27/2011 05:27 PM, Andreas Färber wrote:
Am 26.12.2011 15:58, schrieb Peter Maydell:
diff --git a/hw/sd.c b/hw/sd.c
index 07eb263..2b489d3 100644
--- a/hw/sd.c
+++ b/hw/sd.c

@@ -81,22 +85,22 @@ struct SDState {
     uint8_t sd_status[64];
     uint32_t vhs;
     int wp_switch;
-    int *wp_groups;
+    uint8_t *wp_groups;
     uint64_t size;
-    int blk_len;
+    uint32_t blk_len;
     uint32_t erase_start;
     uint32_t erase_end;
     uint8_t pwd[16];
-    int pwd_len;
-    int function_group[6];
+    uint32_t pwd_len;
+    uint8_t function_group[6];

-    int spi;
-    int current_cmd;
+    uint8_t spi;
+    uint8_t current_cmd;
     /* True if we will handle the next command as an ACMD. Note that this does
      * *not* track the APP_CMD status bit!
-    int expecting_acmd;
-    int blk_written;
+    bool expecting_acmd;

Why have you changed expecting_acmd and enable to bool, but spi
to a uint8_t and wp_groups[] to a uint8_t[] ? This isn't very
consistent. (I'm vaguely against bool because you then end up
making other changes to change '1' to 'true' and so on.)

@@ -1706,5 +1710,44 @@ int sd_data_ready(SDState *sd)

  void sd_enable(SDState *sd, int enable)
-    sd->enable = enable;
+    sd->enable = enable ? true : false;

This kind of thing is why I don't like bool :-)

x = 1 should work with bool when not doing x == true anywhere, here
!!enable would be an alternative. Maybe change int enable to bool, too?


As Peter mentioned previously, we should use either uint8_t or bool for all binary variables for consistency, and we probably shouldn't use bool for wp_groups array since sizeof(bool) uncertainty. So I'm considering just make everything uint8_t.

Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: address@hidden

reply via email to

[Prev in Thread] Current Thread [Next in Thread]