diff mbox series

[2/2] hw/nvme: cleanup error reporting in nvme_init_pci()

Message ID 20221109105357.30430-3-its@irrelevant.dk
State New
Headers show
Series hw/nvme: errp fixes | expand

Commit Message

Klaus Jensen Nov. 9, 2022, 10:53 a.m. UTC 
From: Klaus Jensen <k.jensen@samsung.com>

Replace the local Error variable with errp and ERRP_GUARD().

Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ctrl.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Markus Armbruster Nov. 9, 2022, 12:33 p.m. UTC | #1 
Klaus Jensen <its@irrelevant.dk> writes:

> From: Klaus Jensen <k.jensen@samsung.com>
>
> Replace the local Error variable with errp and ERRP_GUARD().
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>  hw/nvme/ctrl.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index 4cc6ae753295..38eb5ec54f9d 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -7345,13 +7345,13 @@ static int nvme_add_pm_capability(PCIDevice *pci_dev, uint8_t offset)
>  
>  static int nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
>  {
> +    ERRP_GUARD();
> +

Drop the blank line, please.

>      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,13 +7388,13 @@ 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);
> +                    &n->bar0, 0, msix_pba_offset, 0, errp);
>      if (ret < 0) {
>          if (ret == -ENOTSUP) {
> -            warn_report_err(err);
> +            warn_report_err(*errp);
> +            *errp = NULL;
>          } else {
> -            error_propagate(errp, err);
> -            return ret;
> +            return -1;
>          }
>      }

Suggest to reduce nesting:

       if (ret == -ENOTSUP) {
           warn_report_err(*errp);
           *errp = NULL;
       } else if (ret < 0) {
           return -1;
       }

Entirely up to you.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Klaus Jensen Nov. 9, 2022, 12:35 p.m. UTC | #2 
On Nov  9 13:33, 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().
> >
> > Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > ---
> >  hw/nvme/ctrl.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> > index 4cc6ae753295..38eb5ec54f9d 100644
> > --- a/hw/nvme/ctrl.c
> > +++ b/hw/nvme/ctrl.c
> > @@ -7345,13 +7345,13 @@ static int nvme_add_pm_capability(PCIDevice *pci_dev, uint8_t offset)
> >  
> >  static int nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
> >  {
> > +    ERRP_GUARD();
> > +
> 
> Drop the blank line, please.
> 

Roger.

> >      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,13 +7388,13 @@ 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);
> > +                    &n->bar0, 0, msix_pba_offset, 0, errp);
> >      if (ret < 0) {
> >          if (ret == -ENOTSUP) {
> > -            warn_report_err(err);
> > +            warn_report_err(*errp);
> > +            *errp = NULL;
> >          } else {
> > -            error_propagate(errp, err);
> > -            return ret;
> > +            return -1;
> >          }
> >      }
> 
> Suggest to reduce nesting:
> 
>        if (ret == -ENOTSUP) {
>            warn_report_err(*errp);
>            *errp = NULL;
>        } else if (ret < 0) {
>            return -1;
>        }
> 
> Entirely up to you.
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 

Makes sense, thanks!
diff mbox series

Patch

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 4cc6ae753295..38eb5ec54f9d 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -7345,13 +7345,13 @@  static int nvme_add_pm_capability(PCIDevice *pci_dev, uint8_t offset)
 
 static int 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,13 +7388,13 @@  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);
+                    &n->bar0, 0, msix_pba_offset, 0, errp);
     if (ret < 0) {
         if (ret == -ENOTSUP) {
-            warn_report_err(err);
+            warn_report_err(*errp);
+            *errp = NULL;
         } else {
-            error_propagate(errp, err);
-            return ret;
+            return -1;
         }
     }