[syslinux] [PATCH] pci: resize pci_device arrays

[Sebastian Herbszt]

H. Peter Anvin wrote:

>>  
>>  struct pci_bus {
>>  uint16_t id;
>> - struct pci_device *pci_device[MAX_PCI_DEVICES];
>> + struct pci_device *pci_device[MAX_PCI_DEVICES * MAX_PCI_FUNC];
>>  uint8_t pci_device_count;
>>  };
>>  
> 
> I'm still thinking this should be 
> pci_device[MAX_PCI_DEVICES][MAX_PCI_FUNC] to indicate that we're 
> explicitly talking about slots here.

I thought of this patch as one which does "just fix" the current overflow problems.
Since it's simple and doesn't require any "big" code changes it can be done fast so
syslinux doesn't ship with those overflow problems any longer. Later we can change
the pci code to use your pci_device[PCI_DEVICE][FUNC] array structure or linked
lists.

> Also, uint8_t for "count" is unsafe since we can have up to 256 devices. 

uint8_t count can hold values from 0 till 255 and the array can have 256 elements
(0 till 255) so it should be ok.

>  On the other hand, if we use a geographical (uncompacted) 
> representation, perhaps the count is unnecessary.
> 
>>  struct pci_device_list {
>> - struct pci_device pci_device[MAX_PCI_DEVICES];
>> + struct pci_device pci_device[MAX_PCI_BUSES * MAX_PCI_DEVICES * MAX_PCI_FUNC];
>>  uint8_t count;
>>  };
> 
> Using uint8_t for "count" here is even more wrong since we can have up 
> to 65536 devices.

Here you are correct, I have missed this. uint16_t should be enough here, right?
This likely also requires changes to scan.c and maybe some users of pci_device_list.

> Also, each structure like this is over a megabyte in size.  How many of 
> them will we *ever* have at the same time?

The current code only has one, but this array should be changed in the future (s.a).

Should i respin this patch with the missing changes or do you consider this approach not valid?

- Sebastian