Re: [Xen-devel] [PATCH 1/2] xen-pciback: Fix error return in bar_write() and rom_write()

From: Roger Pau MonnÃ
Date: Mon Dec 03 2018 - 07:35:29 EST


On Sun, Dec 02, 2018 at 06:47:32PM +0100, Marek Marczykowski-Górecki wrote:
> From: Dwayne Litzenberger <dlitz@xxxxxxxxx>

I think this requires some description. At least a note that the
function is not altered, just errors from pci reads/writes are no
longer ignored.

> Signed-off-by: Dwayne Litzenberger <dlitz@xxxxxxxxx>
> ---
> drivers/xen/xen-pciback/conf_space_header.c | 24 ++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/xen/xen-pciback/conf_space_header.c b/drivers/xen/xen-pciback/conf_space_header.c
> index 10ae24b..697d0a8 100644
> --- a/drivers/xen/xen-pciback/conf_space_header.c
> +++ b/drivers/xen/xen-pciback/conf_space_header.c
> @@ -135,6 +135,7 @@ static int command_write(struct pci_dev *dev, int offset, u16 value, void *data)
>
> static int rom_write(struct pci_dev *dev, int offset, u32 value, void *data)
> {
> + int err = 0;
> struct pci_bar_info *bar = data;
>
> if (unlikely(!bar)) {
> @@ -150,17 +151,22 @@ static int rom_write(struct pci_dev *dev, int offset, u32 value, void *data)
> bar->which = 1;
> else {
> u32 tmpval;
> - pci_read_config_dword(dev, offset, &tmpval);
> + err = pci_read_config_dword(dev, offset, &tmpval);
> + if (err)
> + goto out;

I don't think you need the out label, you could just return err.
Adding the label doesn't help in any way IMO, just makes the function
one line longer for no reason.

Same for the out label added to bar_write.

Thanks, Roger.