Message ID | 20170207131630.GA28207@mwanda (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On ti, 2017-02-07 at 16:16 +0300, Dan Carpenter wrote: > If "caps.buf" is already NULL then it doesn't need to be freed or set to > NULL. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> <SNIP> > @@ -965,11 +965,8 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd, > sparse->areas[0].offset = > PAGE_ALIGN(vgpu_aperture_offset(vgpu)); > sparse->areas[0].size = vgpu_aperture_sz(vgpu); > - if (!caps.buf) { Looking at the code around, the right thing would be to just remove the negation? This currently seems like a memory leak. > - kfree(caps.buf); > - caps.buf = NULL; > + if (!caps.buf) > caps.size = 0; > - } And quickly looking, the caps is pre-initialized but unused at this point, so the whole if could just be removed, right? Regards, Joonas
On Tue, Feb 07, 2017 at 04:34:57PM +0200, Joonas Lahtinen wrote: > On ti, 2017-02-07 at 16:16 +0300, Dan Carpenter wrote: > > If "caps.buf" is already NULL then it doesn't need to be freed or set to > > NULL. > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > <SNIP> > > > @@ -965,11 +965,8 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd, > > sparse->areas[0].offset = > > PAGE_ALIGN(vgpu_aperture_offset(vgpu)); > > sparse->areas[0].size = vgpu_aperture_sz(vgpu); > > - if (!caps.buf) { > > Looking at the code around, the right thing would be to just remove the > negation? This currently seems like a memory leak. > > > - kfree(caps.buf); > > - caps.buf = NULL; > > + if (!caps.buf) > > caps.size = 0; > > - } > > And quickly looking, the caps is pre-initialized but unused at this > point, so the whole if could just be removed, right? Hm... Duh. You're right. Let me resend. regards, dan carpenter
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 3f656e3a6e5a..de2a55178a37 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -965,11 +965,8 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd, sparse->areas[0].offset = PAGE_ALIGN(vgpu_aperture_offset(vgpu)); sparse->areas[0].size = vgpu_aperture_sz(vgpu); - if (!caps.buf) { - kfree(caps.buf); - caps.buf = NULL; + if (!caps.buf) caps.size = 0; - } break; case VFIO_PCI_BAR3_REGION_INDEX ... VFIO_PCI_BAR5_REGION_INDEX:
If "caps.buf" is already NULL then it doesn't need to be freed or set to NULL. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>