| Message ID | 20240229143914.1977550-7-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.39, 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 vfio_get_group() passes @errp to error_prepend(). Its @errp is > from vfio_attach_device(), which @errp parameter is so widely sourced > that it is necessary to protect it with ERRP_GUARD(). > > To avoid the issue like [1] said, 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: Alex Williamson <alex.williamson@redhat.com> > Cc: "Cédric Le Goater" <clg@redhat.com> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > --- > hw/vfio/container.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/vfio/container.c b/hw/vfio/container.c > index bd25b9fbad2e..f66bb01f5b18 100644 > --- a/hw/vfio/container.c > +++ b/hw/vfio/container.c > @@ -719,6 +719,7 @@ static void vfio_disconnect_container(VFIOGroup *group) > > static VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp) > { > + ERRP_GUARD(); > VFIOGroup *group; > char path[32]; > struct vfio_group_status status = { .argsz = sizeof(status) }; Reviewed-by: Thomas Huth <thuth@redhat.com>
Hello, On 2/29/24 15:39, 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 vfio_get_group() passes @errp to error_prepend(). Its @errp is > from vfio_attach_device(), which @errp parameter is so widely sourced > that it is necessary to protect it with ERRP_GUARD(). > > To avoid the issue like [1] said, 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: Alex Williamson <alex.williamson@redhat.com> > Cc: "Cédric Le Goater" <clg@redhat.com> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C.
diff --git a/hw/vfio/container.c b/hw/vfio/container.c index bd25b9fbad2e..f66bb01f5b18 100644 --- a/hw/vfio/container.c +++ b/hw/vfio/container.c @@ -719,6 +719,7 @@ static void vfio_disconnect_container(VFIOGroup *group) static VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp) { + ERRP_GUARD(); VFIOGroup *group; char path[32]; struct vfio_group_status status = { .argsz = sizeof(status) };