Re: [PATCH] pci: Use same logic in pci_vpd_read as that of pci_vpd_write

[Bjorn Helgaas <helgaas@kernel.org>]

Hi Casey,

Sorry for the delay in responding.

On Mon, Aug 08, 2016 at 09:20:22PM +0000, Casey Leedom wrote:
> | From: Bjorn Helgaas <helgaas@kernel.org>
> | 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.

Drivers that read VPD probably can figure out how much to read.  The
place I'm worried about is read_vpd_attr(), the hook for reading VPD
via sysfs.  If we do something like "cat /sys/.../vpd", I think cat
will try to read a large chunk and it will currently get a short read.
If we return an error instead of the short read, I think the sysfs
file will be much harder to use.

Bjorn