| Message ID | 20240229143914.1977550-3-zhao1.liu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleanup up to fix missing ERRP_GUARD() for error_prepend() | expand |
On 29/02/2024 15.38, Zhao Liu wrote: > From: Zhao Liu <zhao1.liu@intel.com> > > As the comment in qapi/error, passing @errp to error_prepend() requires > ERRP_GUARD(): > > * = Why, when and how to use ERRP_GUARD() = > * > * Without ERRP_GUARD(), use of the @errp parameter is restricted: > ... > * - It should not be passed to error_prepend(), error_vprepend() or > * error_append_hint(), because that doesn't work with &error_fatal. > * ERRP_GUARD() lifts these restrictions. > * > * To use ERRP_GUARD(), add it right at the beginning of the function. > * @errp can then be used without worrying about the argument being > * NULL or &error_fatal. > > ERRP_GUARD() could avoid the case when @errp is the pointer of > error_fatal, the user can't see this additional information, because > exit() happens in error_setg earlier than information is added [1]. > > The xen_netdev_connect() passes @errp to error_prepend(), and its @errp > parameter is from xen_device_frontend_changed(). > > Though its @errp points to @local_err of xen_device_frontend_changed(), > to follow the requirement of @errp, add missing ERRP_GUARD() at the > beginning of this function. > > [1]: Issue description in the commit message of commit ae7c80a7bd73 > ("error: New macro ERRP_GUARD()"). > > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Anthony Perard <anthony.perard@citrix.com> > Cc: Paul Durrant <paul@xen.org> > Cc: Jason Wang <jasowang@redhat.com> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > --- > hw/net/xen_nic.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c > index 453fdb981983..89487b49baf9 100644 > --- a/hw/net/xen_nic.c > +++ b/hw/net/xen_nic.c > @@ -351,6 +351,7 @@ static bool net_event(void *_xendev) > > static bool xen_netdev_connect(XenDevice *xendev, Error **errp) > { > + ERRP_GUARD(); > XenNetDev *netdev = XEN_NET_DEVICE(xendev); > unsigned int port, rx_copy; > Reviewed-by: Thomas Huth <thuth@redhat.com>
On Thu, Feb 29, 2024 at 06:25:40PM +0100, Thomas Huth wrote: > On 29/02/2024 15.38, Zhao Liu wrote: > > From: Zhao Liu <zhao1.liu@intel.com> > > > > As the comment in qapi/error, passing @errp to error_prepend() requires > > ERRP_GUARD(): > > > > * = Why, when and how to use ERRP_GUARD() = > > * > > * Without ERRP_GUARD(), use of the @errp parameter is restricted: > > ... > > * - It should not be passed to error_prepend(), error_vprepend() or > > * error_append_hint(), because that doesn't work with &error_fatal. > > * ERRP_GUARD() lifts these restrictions. > > * > > * To use ERRP_GUARD(), add it right at the beginning of the function. > > * @errp can then be used without worrying about the argument being > > * NULL or &error_fatal. > > > > ERRP_GUARD() could avoid the case when @errp is the pointer of > > error_fatal, the user can't see this additional information, because > > exit() happens in error_setg earlier than information is added [1]. > > > > The xen_netdev_connect() passes @errp to error_prepend(), and its @errp > > parameter is from xen_device_frontend_changed(). > > > > Though its @errp points to @local_err of xen_device_frontend_changed(), > > to follow the requirement of @errp, add missing ERRP_GUARD() at the > > beginning of this function. > > > > [1]: Issue description in the commit message of commit ae7c80a7bd73 > > ("error: New macro ERRP_GUARD()"). > > > > Cc: Stefano Stabellini <sstabellini@kernel.org> > > Cc: Anthony Perard <anthony.perard@citrix.com> > > Cc: Paul Durrant <paul@xen.org> > > Cc: Jason Wang <jasowang@redhat.com> > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > > --- > > hw/net/xen_nic.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c > > index 453fdb981983..89487b49baf9 100644 > > --- a/hw/net/xen_nic.c > > +++ b/hw/net/xen_nic.c > > @@ -351,6 +351,7 @@ static bool net_event(void *_xendev) > > static bool xen_netdev_connect(XenDevice *xendev, Error **errp) > > { > > + ERRP_GUARD(); > > XenNetDev *netdev = XEN_NET_DEVICE(xendev); > > unsigned int port, rx_copy; > > Reviewed-by: Thomas Huth <thuth@redhat.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com> Thanks,
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c index 453fdb981983..89487b49baf9 100644 --- a/hw/net/xen_nic.c +++ b/hw/net/xen_nic.c @@ -351,6 +351,7 @@ static bool net_event(void *_xendev) static bool xen_netdev_connect(XenDevice *xendev, Error **errp) { + ERRP_GUARD(); XenNetDev *netdev = XEN_NET_DEVICE(xendev); unsigned int port, rx_copy;