| Message ID | 20200529214807.3801851-2-emil.l.velikov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm: vmwgfx: remove drm_driver::master_set() return typ | expand |
Hi Emil. On Fri, May 29, 2020 at 10:48:07PM +0100, Emil Velikov wrote: > The variables are already the exact same value or will be overwritten > shortly afterwords. In either case there's no functional difference. s/afterwords/afterwards/ > > Cc: David Airlie <airlied@linux.ie> > Cc: Daniel Vetter <daniel@ffwll.ch> > Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> > --- > drivers/gpu/drm/drm_auth.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c > index db701a9e9393..5ae5623f2482 100644 > --- a/drivers/gpu/drm/drm_auth.c > +++ b/drivers/gpu/drm/drm_auth.c > @@ -215,7 +215,7 @@ drm_master_check_perm(struct drm_device *dev, struct drm_file *file_priv) > int drm_setmaster_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_priv) > { > - int ret = 0; > + int ret; This was the redundant asignment I mentioned in first mail - good. > > mutex_lock(&dev->master_mutex); > > @@ -282,7 +282,6 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data, > > if (file_priv->master->lessor != NULL) { > DRM_DEBUG_LEASE("Attempt to drop lessee %d as master\n", file_priv->master->lessee_id); > - ret = -EINVAL; This is wrong. ret is 0 when this code is reached, so we loose the error value if this code-path is triggered. Or I miss something?? Sam > goto out_unlock; > } > > -- > 2.25.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Sat, 30 May 2020 at 08:48, Sam Ravnborg <sam@ravnborg.org> wrote: > > Hi Emil. > > On Fri, May 29, 2020 at 10:48:07PM +0100, Emil Velikov wrote: > > The variables are already the exact same value or will be overwritten > > shortly afterwords. In either case there's no functional difference. > s/afterwords/afterwards/ > > > > > Cc: David Airlie <airlied@linux.ie> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> > > --- > > drivers/gpu/drm/drm_auth.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c > > index db701a9e9393..5ae5623f2482 100644 > > --- a/drivers/gpu/drm/drm_auth.c > > +++ b/drivers/gpu/drm/drm_auth.c > > @@ -215,7 +215,7 @@ drm_master_check_perm(struct drm_device *dev, struct drm_file *file_priv) > > int drm_setmaster_ioctl(struct drm_device *dev, void *data, > > struct drm_file *file_priv) > > { > > - int ret = 0; > > + int ret; > > This was the redundant asignment I mentioned in first mail - good. > > > > mutex_lock(&dev->master_mutex); > > > > @@ -282,7 +282,6 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data, > > > > if (file_priv->master->lessor != NULL) { > > DRM_DEBUG_LEASE("Attempt to drop lessee %d as master\n", file_priv->master->lessee_id); > > - ret = -EINVAL; > This is wrong. ret is 0 when this code is reached, so we loose the error > value if this code-path is triggered. > Or I miss something?? > A few lines above [1] - there's unconditional ret = -EINVAL. Although the set<>drop paths are pretty asymmetric and misleading. Let me respin the series. -Emil [1] https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/drm_auth.c?id=6015002ece38dac85a373f041e0302781de7474b#n293
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index db701a9e9393..5ae5623f2482 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -215,7 +215,7 @@ drm_master_check_perm(struct drm_device *dev, struct drm_file *file_priv) int drm_setmaster_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { - int ret = 0; + int ret; mutex_lock(&dev->master_mutex); @@ -282,7 +282,6 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data, if (file_priv->master->lessor != NULL) { DRM_DEBUG_LEASE("Attempt to drop lessee %d as master\n", file_priv->master->lessee_id); - ret = -EINVAL; goto out_unlock; }
The variables are already the exact same value or will be overwritten shortly afterwords. In either case there's no functional difference. Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> --- drivers/gpu/drm/drm_auth.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)