On 9 January 2013 08:45, KONRAD Frédéric <fred.kon...@greensocs.com> wrote:
> On 08/01/2013 18:54, Peter Maydell wrote:
>> This doesn't have any code in it yet (it gets added in later patches) but
>> the final result looks very repetitive. I think you'd be better off just
>> having
>> some arrays:
>>
>> uint16_t virtio_pci_device_id[] = {
>> [VIRTIO_ID_BLOCK] = PCI_DEVICE_ID_VIRTIO_BLOCK,
>> [VIRTIO_ID_NET] = PCI_DEVICE_ID_VIRTIO_NET,
>> (etc)
>> };
>>
>> similarly for the class. Then you can just drop the switch statement.
>>
>> In fact I think you might as well put in the array entries for all
>> the virtio devices in this patch rather than adding one in each of the
>> "add virtio-foo device" patches; it will do no harm for them to be
>> there early and it makes the later patches a little smaller.
>
> And what happen when it is a bad ID ?
Error, same as before. Obviously you protect the array lookup
with a sizeof check for out of range, and you know if the array
entry is 0 it's invalid (0 being neither a valid device id or class).
If you think that looks too ugly then you could keep the switch, but
don't put calls to pci_config_set_device_id() and pci_config_set_class()
in each switch, that's just code duplication.
-- PMM
