From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:43821) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9aSu-0004Rk-OL for qemu-devel@nongnu.org; Tue, 12 Apr 2011 06:04:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q9aSs-0004Tp-Lg for qemu-devel@nongnu.org; Tue, 12 Apr 2011 06:04:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:26990) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q9aSs-0004TU-84 for qemu-devel@nongnu.org; Tue, 12 Apr 2011 06:04:54 -0400 Message-ID: <4DA42453.2050707@redhat.com> Date: Tue, 12 Apr 2011 12:07:15 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <92999599c35549934c2712b4159d6b3411c61f49.1302600061.git.amit.shah@redhat.com> In-Reply-To: <92999599c35549934c2712b4159d6b3411c61f49.1302600061.git.amit.shah@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 4/6] atapi: GESN: Use structs for commonly-used field types List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amit Shah Cc: Juan Quintela , Stefan Hajnoczi , Markus Armbruster , qemu list , Paolo Bonzini Am 12.04.2011 11:27, schrieb Amit Shah: > Instead of using magic numbers, use structs that are more descriptive of > the fields being used. > > Signed-off-by: Amit Shah > --- > hw/ide/core.c | 15 +++++++++++++-- > 1 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/hw/ide/core.c b/hw/ide/core.c > index c5913d8..fe50d8a 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -1097,12 +1097,23 @@ static void handle_get_event_status_notification(IDEState *s, > uint8_t *buf, > const uint8_t *packet) > { > + struct { > + uint8_t opcode; > + uint8_t polled; /* lsb bit is polled; others are reserved */ > + uint8_t reserved2[2]; > + uint8_t request; Maybe class would be a better name? > + uint8_t reserved3[2]; > + uint8_t len_msb; > + uint8_t len_lsb; Why not a uint16_t length? > + uint8_t control; > + } __attribute__((packed)) *gesn_cdb; > unsigned int max_len, used_len; > > - max_len = ube16_to_cpu(packet + 7); > + gesn_cdb = (void *)packet; > + max_len = ube16_to_cpu(&gesn_cdb->len_msb); Instead of reading 16 bit from a 8 bit pointer, you could then use be16_to_cpu(gesn_cdb->length); > > /* It is fine by the MMC spec to not support async mode operations */ > - if (!(packet[1] & 0x01)) { /* asynchronous mode */ > + if (!(gesn_cdb->polled & 0x01)) { /* asynchronous mode */ > /* Only polling is supported, asynchronous mode is not. */ > ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST, > ASC_INV_FIELD_IN_CMD_PACKET); Kevin