[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/2] hw/nvme: cleanup error reporting in nvme_init_pci()
From: |
Klaus Jensen |
Subject: |
Re: [PATCH v2 2/2] hw/nvme: cleanup error reporting in nvme_init_pci() |
Date: |
Thu, 10 Nov 2022 23:06:19 +0100 |
On Nov 10 11:00, Markus Armbruster wrote:
> Klaus Jensen <its@irrelevant.dk> writes:
>
> > From: Klaus Jensen <k.jensen@samsung.com>
> >
> > Replace the local Error variable with errp and ERRP_GUARD() and change
> > the return value to bool.
> >
> > Reviewed-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > ---
> > hw/nvme/ctrl.c | 23 ++++++++++-------------
> > 1 file changed, 10 insertions(+), 13 deletions(-)
> >
> > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> > index a5c0a5fa6ce2..e23af4db91ae 100644
> > --- a/hw/nvme/ctrl.c
> > +++ b/hw/nvme/ctrl.c
> > @@ -7343,15 +7343,14 @@ static int nvme_add_pm_capability(PCIDevice
> > *pci_dev, uint8_t offset)
> > return 0;
> > }
> >
> > -static int nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
> > +static bool nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
> > {
> > + ERRP_GUARD();
> > uint8_t *pci_conf = pci_dev->config;
> > uint64_t bar_size;
> > unsigned msix_table_offset, msix_pba_offset;
> > int ret;
> >
> > - Error *err = NULL;
> > -
> > pci_conf[PCI_INTERRUPT_PIN] = 1;
> > pci_config_set_prog_interface(pci_conf, 0x2);
> >
> > @@ -7388,14 +7387,12 @@ static int nvme_init_pci(NvmeCtrl *n, PCIDevice
> > *pci_dev, Error **errp)
> > }
> > ret = msix_init(pci_dev, n->params.msix_qsize,
> > &n->bar0, 0, msix_table_offset,
> > - &n->bar0, 0, msix_pba_offset, 0, &err);
> > - if (ret < 0) {
> > - if (ret == -ENOTSUP) {
> > - warn_report_err(err);
> > - } else {
> > - error_propagate(errp, err);
> > - return ret;
> > - }
> > + &n->bar0, 0, msix_pba_offset, 0, errp);
> > + if (ret == -ENOTSUP) {
> > + warn_report_err(*errp);
> > + *errp = NULL;
> > + } else {
>
> Uh, shouldn't this be
>
> } else if (ret < 0) {
>
> ?
>
Yikes. Yes.
signature.asc
Description: PGP signature