Am 19.04.2011 15:20, schrieb Amit Shah:
> On (Tue) 19 Apr 2011 [14:36:41], Kevin Wolf wrote:
> 
>> +struct {
>> +    void (*handler)(IDEState *s, uint8_t *buf);
>> +    int flags;
>> +} atapi_cmd_table[0x100] = {
>> +    [ 0x00 ] = { cmd_test_unit_ready,               0 },
>> +    [ 0x03 ] = { cmd_request_sense,                 ALLOW_UA },
>> +    [ 0x12 ] = { cmd_inquiry,                       ALLOW_UA },
>> +    [ 0x1a ] = { cmd_mode_sense, /* (6) */          0 },
>> +    [ 0x1b ] = { cmd_start_stop_unit,               0 },
>> +    [ 0x1e ] = { cmd_prevent_allow_medium_removal,  0 },
>> +    [ 0x25 ] = { cmd_read_cdvd_capacity,            0 },
>> +    [ 0x28 ] = { cmd_read, /* (10) */               0 },
>> +    [ 0x2b ] = { cmd_seek,                          0 },
>> +    [ 0x43 ] = { cmd_read_toc_pma_atip,             0 },
>> +    [ 0x46 ] = { cmd_get_configuration,             ALLOW_UA },
>> +    [ 0x4a ] = { cmd_get_event_status_notification, ALLOW_UA },
>> +    [ 0x5a ] = { cmd_mode_sense, /* (10) */         0 },
>> +    [ 0xa8 ] = { cmd_read, /* (12) */               0 },
>> +    [ 0xad ] = { cmd_read_dvd_structure,            0 },
>> +    [ 0xbb ] = { cmd_set_speed,                     0 },
>> +    [ 0xbd ] = { cmd_mechanism_status,              0 },
>> +    [ 0xbe ] = { cmd_read_cd,                       0 },
>> +};
> 
> I'd prefer to use the GPCMD_TEST_UNIT_READY, etc., defines we already
> have in internal.h instead of using the command numbers here.

In fact, I was considering to remove the GPCMD_* definitions because
most of them are unused now (only cmd_mode_sense/cmd_read still use them
to distinguish the variants).

What I like about this table is that you have all information about a
command directly visible: Its opcode, its name and any flags. Using the
constants would hide the opcode again, duplicate the name and probably
make the lines longer than 80 characters.

Kevin

Reply via email to