| Message ID | 20240229143914.1977550-10-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 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]. > > In hw/vfio/pci-quirks.c, there're 2 functions passing @errp to > error_prepend() without ERRP_GUARD(): > - vfio_add_nv_gpudirect_cap() > - vfio_add_vmd_shadow_cap() > > Their @errp parameters are so widely sourced that it is necessary to > protect them 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. > --- > hw/vfio/pci-quirks.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c > index 84b1a7b9485c..496fd1ee86bd 100644 > --- a/hw/vfio/pci-quirks.c > +++ b/hw/vfio/pci-quirks.c > @@ -1538,6 +1538,7 @@ static bool is_valid_std_cap_offset(uint8_t pos) > > static int vfio_add_nv_gpudirect_cap(VFIOPCIDevice *vdev, Error **errp) > { > + ERRP_GUARD(); > PCIDevice *pdev = &vdev->pdev; > int ret, pos; > bool c8_conflict = false, d4_conflict = false; > @@ -1630,6 +1631,7 @@ static int vfio_add_nv_gpudirect_cap(VFIOPCIDevice *vdev, Error **errp) > #define VMD_SHADOW_CAP_LEN 24 > static int vfio_add_vmd_shadow_cap(VFIOPCIDevice *vdev, Error **errp) > { > + ERRP_GUARD(); > uint8_t membar_phys[16]; > int ret, pos = 0xE8; >
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c index 84b1a7b9485c..496fd1ee86bd 100644 --- a/hw/vfio/pci-quirks.c +++ b/hw/vfio/pci-quirks.c @@ -1538,6 +1538,7 @@ static bool is_valid_std_cap_offset(uint8_t pos) static int vfio_add_nv_gpudirect_cap(VFIOPCIDevice *vdev, Error **errp) { + ERRP_GUARD(); PCIDevice *pdev = &vdev->pdev; int ret, pos; bool c8_conflict = false, d4_conflict = false; @@ -1630,6 +1631,7 @@ static int vfio_add_nv_gpudirect_cap(VFIOPCIDevice *vdev, Error **errp) #define VMD_SHADOW_CAP_LEN 24 static int vfio_add_vmd_shadow_cap(VFIOPCIDevice *vdev, Error **errp) { + ERRP_GUARD(); uint8_t membar_phys[16]; int ret, pos = 0xE8;