From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:51952) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RfXUf-0007Ha-Kq for qemu-devel@nongnu.org; Tue, 27 Dec 2011 08:55:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RfXUd-00058X-Di for qemu-devel@nongnu.org; Tue, 27 Dec 2011 08:55:05 -0500 Received: from mailout4.w1.samsung.com ([210.118.77.14]:52354) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RfXUc-00057h-TN for qemu-devel@nongnu.org; Tue, 27 Dec 2011 08:55:03 -0500 Received: from euspt1 ([210.118.77.14]) by mailout4.w1.samsung.com (Sun Java(tm) System Messaging Server 6.3-8.04 (built Jul 29 2009; 32bit)) with ESMTP id <0LWV00IZ07ZMPO40@mailout4.w1.samsung.com> for qemu-devel@nongnu.org; Tue, 27 Dec 2011 13:54:58 +0000 (GMT) Received: from [106.109.8.52] by spt1.w1.samsung.com (iPlanet Messaging Server 5.2 Patch 2 (built Jul 14 2004)) with ESMTPA id <0LWV00HKO7ZLA2@spt1.w1.samsung.com> for qemu-devel@nongnu.org; Tue, 27 Dec 2011 13:54:58 +0000 (GMT) Date: Tue, 27 Dec 2011 17:54:57 +0400 From: Mitsyanko Igor In-reply-to: <4EF9C7CB.4020007@suse.de> Message-id: <4EF9CE31.2090504@samsung.com> MIME-version: 1.0 Content-type: text/plain; charset=UTF-8; format=flowed Content-transfer-encoding: QUOTED-PRINTABLE References: <1324893824-13558-1-git-send-email-i.mitsyanko@samsung.com> <1324893824-13558-3-git-send-email-i.mitsyanko@samsung.com> <4EF9C7CB.4020007@suse.de> Subject: Re: [Qemu-devel] [PATCH 2/3] hw/sd.c: add SD card save/load support Reply-To: i.mitsyanko@samsung.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Peter Maydell , e.voevodin@samsung.com, Juan Quintela , kyungmin.park@samsung.com, d.solodkiy@samsung.com, m.kozlov@samsung.com, jehyung.lee@samsung.com, =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= On 12/27/2011 05:27 PM, Andreas F=C3=A4rber 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 =3D enable; >>> + sd->enable =3D enable ? true : false; >> >> This kind of thing is why I don't like bool :-) > > x =3D 1 should work with bool when not doing x =3D=3D true anywhere= , here > !!enable would be an alternative. Maybe change int enable to bool, = too? > > Andreas > As Peter mentioned previously, we should use either uint8_t or bool f= or=20 all binary variables for consistency, and we probably shouldn't use b= ool=20 for wp_groups array since sizeof(bool) uncertainty. So I'm considerin= g=20 just make everything uint8_t. --=20 Mitsyanko Igor ASWG, Moscow R&D center, Samsung Electronics email: i.mitsyanko@samsung.com