| From: Bjorn Helgaas <helgaas@xxxxxxxxxx> | Sent: Monday, August 8, 2016 2:08 PM | | I think the existing semantics are the same as for the read(2) | syscall: we return the number of bytes read, which may be less than | the size requested, and callers may use random garbage if they don't | check for short reads. read(2) is a generic I/O system call, which maintains a read pointer with lseek(2) semantics, etc. This seems like an Apples and Oranges comparison. | If we make pci_read_vpd() return error instead of a short read, how do | callers figure out how much to request? The same question could be asked of the pci_write_vpd() call which currently throws an error is any portion of the requested write is beyond the recorded VPD area. Also, isn't this what (struct pci_dev *)->vpd.len is for? Although I think we'd want an API to retrieve that rather than have callers simply access the potentially uninitialized field which is lazily initialized. On the other hand, we don't seem to have any current callers with this issue. | In the current tree, I think the following callers don't handle short | reads correctly: | | cxl_pci_read_adapter_vpd() (cxl, used via read_vpd()) | eeprom_rd_phys() (cxgb4) | t4_get_raw_vpd_params() (cxgb4) | sky2_show_vpd() (sky2) | efx_probe_vpd_strings() (efx) | vfio_vpd_config_write() (vfio) | | That's not a very long list, so we could certainly fix them. I agree. If we do decide not to address the weirdness of returning success for a partial VPD read, then at least the fixup of the callers isn't too bad. Casey-- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
