[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/5] ide/atapi: Use table instead of switch f
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/5] ide/atapi: Use table instead of switch for commands |
Date: |
Thu, 21 Apr 2011 10:13:43 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101027 Fedora/3.0.10-1.fc12 Thunderbird/3.0.10 |
Am 20.04.2011 20:13, schrieb Blue Swirl:
> On Wed, Apr 20, 2011 at 2:30 PM, Kevin Wolf <address@hidden> wrote:
>> Signed-off-by: Kevin Wolf <address@hidden>
>> ---
>> hw/ide/atapi.c | 115
>> +++++++++++++++++++++++--------------------------------
>> 1 files changed, 48 insertions(+), 67 deletions(-)
>>
>> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
>> index d161bf7..d0bf7fd 100644
>> --- a/hw/ide/atapi.c
>> +++ b/hw/ide/atapi.c
>> @@ -533,10 +533,11 @@ static unsigned int event_status_media(IDEState *s,
>> return 8; /* We wrote to 4 extra bytes from the header */
>> }
>>
>> -static void handle_get_event_status_notification(IDEState *s,
>> - uint8_t *buf,
>> - const uint8_t *packet)
>> +static void cmd_get_event_status_notification(IDEState *s,
>> + uint8_t *buf)
>> {
>> + const uint8_t *packet = buf;
>> +
>> struct {
>> uint8_t opcode;
>> uint8_t polled; /* lsb bit is polled; others are reserved */
>> @@ -1064,6 +1065,38 @@ static void cmd_set_speed(IDEState *s, uint8_t* buf)
>> ide_atapi_cmd_ok(s);
>> }
>>
>> +enum {
>> + /*
>> + * Only commands flagged as ALLOW_UA are allowed to run under a
>> + * unit attention condition. (See MMC-5, section 4.1.6.1)
>> + */
>> + ALLOW_UA = 0x01,
>> +};
>> +
>> +struct {
>> + void (*handler)(IDEState *s, uint8_t *buf);
>> + int flags;
>> +} atapi_cmd_table[0x100] = {
>> + [ 0x00 ] = { cmd_test_unit_ready, 0 },
>
> How about using symbols here, like
> [ GPCMD_TEST_UNIT_READY ] = { cmd_test_unit_ready, 0 },
> ?
I avoided symbols intentionally. In fact, I'm even considering to remove
the symbols from the header file because most of them are unused now
(and it should be easy enough to get rid of the rest of them so that the
only occurrence of the literal opcode is in the table).
The point here is that this table contains all the information about a
command in one place. If you used symbols, you would have the command
name twice, and the opcode would be missing. So not using symbolic
constants but rather literal opcodes improves readability in this case.
> The table can probably be static const.
Right, will fix.
Kevin
- [Qemu-devel] [PATCH v2 0/5] atapi: Some code restructuring, Kevin Wolf, 2011/04/20
- [Qemu-devel] [PATCH v2 3/5] ide/atapi: Use table instead of switch for commands, Kevin Wolf, 2011/04/20
- [Qemu-devel] [PATCH v2 5/5] ide/atapi: Introduce CHECK_READY flag for commands, Kevin Wolf, 2011/04/20
- [Qemu-devel] [PATCH v2 2/5] ide/atapi: Factor commands out, Kevin Wolf, 2011/04/20
- [Qemu-devel] [PATCH v2 4/5] ide/atapi: Replace bdrv_get_geometry calls by s->nb_sectors, Kevin Wolf, 2011/04/20
- [Qemu-devel] [PATCH v2 1/5] ide: Split atapi.c out, Kevin Wolf, 2011/04/20