| Message ID | 156871568761.196432.13197720535866413065.stgit@bahia.lan |
|---|---|
| State | New |
| Headers | show |
| Series | Fix usage of error_append_hint() | expand |
On Tue, Sep 17, 2019 at 12:21:27PM +0200, Greg Kurz wrote: > Commit 4d71b38ae8fa converted many error_setg() call sites to > rdma_error_report(), but it forgot to convert a companion > error_append_hint(). Since no guy doesn't set errp anymore in > pvrdma_realize(), errp remains NULL and error_append_hint() does > nothing. > > Also error_append_hint() was a poor choice since its "intended use > is adding helpful hints on the human user interface" and "not for > clarifying a confusing error message". > > Call rdma_error_report() instead. Thanks, So are you suggesting to replace all other error_setg calls with rdma_error_report instead? > > Fixes: 4d71b38ae8fa "hw/rdma: Switch to generic error reporting way" > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > hw/rdma/vmw/pvrdma_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c > index 3e36e130139c..d370ae07ca6a 100644 > --- a/hw/rdma/vmw/pvrdma_main.c > +++ b/hw/rdma/vmw/pvrdma_main.c > @@ -667,7 +667,7 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp) > out: > if (rc) { > pvrdma_fini(pdev); > - error_append_hint(errp, "Device failed to load\n"); > + rdma_error_report("Device failed to load"); Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com> > } > } > > >
On Tue, 17 Sep 2019 17:51:57 +0300 Yuval Shaia <yuval.shaia@oracle.com> wrote: > On Tue, Sep 17, 2019 at 12:21:27PM +0200, Greg Kurz wrote: > > Commit 4d71b38ae8fa converted many error_setg() call sites to > > rdma_error_report(), but it forgot to convert a companion > > error_append_hint(). Since no guy doesn't set errp anymore in > > pvrdma_realize(), errp remains NULL and error_append_hint() does > > nothing. > > > > Also error_append_hint() was a poor choice since its "intended use > > is adding helpful hints on the human user interface" and "not for > > clarifying a confusing error message". > > > > Call rdma_error_report() instead. > > Thanks, > So are you suggesting to replace all other error_setg calls with > rdma_error_report instead? > No. I don't know what was the motivation behind 4d71b38ae8fa, I'm just fixing what seems to be a leftover, which should have been error_prepend() instead of error_append_hint() actually. > > > > Fixes: 4d71b38ae8fa "hw/rdma: Switch to generic error reporting way" > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > hw/rdma/vmw/pvrdma_main.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c > > index 3e36e130139c..d370ae07ca6a 100644 > > --- a/hw/rdma/vmw/pvrdma_main.c > > +++ b/hw/rdma/vmw/pvrdma_main.c > > @@ -667,7 +667,7 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp) > > out: > > if (rc) { > > pvrdma_fini(pdev); > > - error_append_hint(errp, "Device failed to load\n"); > > + rdma_error_report("Device failed to load"); > > Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com> > > > } > > } > > > > > >
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c index 3e36e130139c..d370ae07ca6a 100644 --- a/hw/rdma/vmw/pvrdma_main.c +++ b/hw/rdma/vmw/pvrdma_main.c @@ -667,7 +667,7 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp) out: if (rc) { pvrdma_fini(pdev); - error_append_hint(errp, "Device failed to load\n"); + rdma_error_report("Device failed to load"); } }
Commit 4d71b38ae8fa converted many error_setg() call sites to rdma_error_report(), but it forgot to convert a companion error_append_hint(). Since no guy doesn't set errp anymore in pvrdma_realize(), errp remains NULL and error_append_hint() does nothing. Also error_append_hint() was a poor choice since its "intended use is adding helpful hints on the human user interface" and "not for clarifying a confusing error message". Call rdma_error_report() instead. Fixes: 4d71b38ae8fa "hw/rdma: Switch to generic error reporting way" Signed-off-by: Greg Kurz <groug@kaod.org> --- hw/rdma/vmw/pvrdma_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)