* [PATCH linux-next] drm/gem: Remove the unneeded result variable @ 2022-08-31 3:33 ` cgel.zte 0 siblings, 0 replies; 6+ messages in thread From: cgel.zte @ 2022-08-31 3:33 UTC (permalink / raw) To: airlied; +Cc: tzimmermann, ye xingchen, Zeal Robot, linux-kernel, dri-devel From: ye xingchen <ye.xingchen@zte.com.cn> Return the value drm_gem_handle_delete() directly instead of storing it in another redundant variable. Reported-by: Zeal Robot <zealci@zte.com.cn> Signed-off-by: ye xingchen <ye.xingchen@zte.com.cn> --- drivers/gpu/drm/drm_gem.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index ad068865ba20..3fa0deff3014 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -782,14 +782,11 @@ drm_gem_close_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_gem_close *args = data; - int ret; if (!drm_core_check_feature(dev, DRIVER_GEM)) return -EOPNOTSUPP; - ret = drm_gem_handle_delete(file_priv, args->handle); - - return ret; + return drm_gem_handle_delete(file_priv, args->handle); } /** -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH linux-next] drm/gem: Remove the unneeded result variable @ 2022-08-31 3:33 ` cgel.zte 0 siblings, 0 replies; 6+ messages in thread From: cgel.zte @ 2022-08-31 3:33 UTC (permalink / raw) To: airlied Cc: maarten.lankhorst, mripard, tzimmermann, daniel, dri-devel, linux-kernel, ye xingchen, Zeal Robot From: ye xingchen <ye.xingchen@zte.com.cn> Return the value drm_gem_handle_delete() directly instead of storing it in another redundant variable. Reported-by: Zeal Robot <zealci@zte.com.cn> Signed-off-by: ye xingchen <ye.xingchen@zte.com.cn> --- drivers/gpu/drm/drm_gem.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index ad068865ba20..3fa0deff3014 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -782,14 +782,11 @@ drm_gem_close_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_gem_close *args = data; - int ret; if (!drm_core_check_feature(dev, DRIVER_GEM)) return -EOPNOTSUPP; - ret = drm_gem_handle_delete(file_priv, args->handle); - - return ret; + return drm_gem_handle_delete(file_priv, args->handle); } /** -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH linux-next] drm/gem: Remove the unneeded result variable 2022-08-31 3:33 ` cgel.zte @ 2022-08-31 9:25 ` Jani Nikula -1 siblings, 0 replies; 6+ messages in thread From: Jani Nikula @ 2022-08-31 9:25 UTC (permalink / raw) To: cgel.zte, airlied Cc: ye xingchen, Zeal Robot, linux-kernel, dri-devel, Julia Lawall, Wolfram Sang, Mark Brown, Maxime Ripard, tzimmermann, Thierry Reding On Wed, 31 Aug 2022, cgel.zte@gmail.com wrote: > From: ye xingchen <ye.xingchen@zte.com.cn> > > Return the value drm_gem_handle_delete() directly instead of storing it > in another redundant variable. Personally, I generally prefer keeping these types of variables, because they make future changes on the code easier. For example, adding a new call where you need to check the return value, or having to deal with the error value right there. Or just adding ad hoc debug logging of the return value. Related, but not exactly the same, see commit b784c7707502 ("coccinnelle: Remove ptr_ret script"). Adding Cc's from that commit for input. BR, Jani. > Reported-by: Zeal Robot <zealci@zte.com.cn> > Signed-off-by: ye xingchen <ye.xingchen@zte.com.cn> > --- > drivers/gpu/drm/drm_gem.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index ad068865ba20..3fa0deff3014 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -782,14 +782,11 @@ drm_gem_close_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_priv) > { > struct drm_gem_close *args = data; > - int ret; > > if (!drm_core_check_feature(dev, DRIVER_GEM)) > return -EOPNOTSUPP; > > - ret = drm_gem_handle_delete(file_priv, args->handle); > - > - return ret; > + return drm_gem_handle_delete(file_priv, args->handle); > } > > /** -- Jani Nikula, Intel Open Source Graphics Center ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH linux-next] drm/gem: Remove the unneeded result variable @ 2022-08-31 9:25 ` Jani Nikula 0 siblings, 0 replies; 6+ messages in thread From: Jani Nikula @ 2022-08-31 9:25 UTC (permalink / raw) To: cgel.zte, airlied Cc: tzimmermann, ye xingchen, Zeal Robot, linux-kernel, dri-devel, Maxime Ripard, Thierry Reding, Julia Lawall, Wolfram Sang, Mark Brown On Wed, 31 Aug 2022, cgel.zte@gmail.com wrote: > From: ye xingchen <ye.xingchen@zte.com.cn> > > Return the value drm_gem_handle_delete() directly instead of storing it > in another redundant variable. Personally, I generally prefer keeping these types of variables, because they make future changes on the code easier. For example, adding a new call where you need to check the return value, or having to deal with the error value right there. Or just adding ad hoc debug logging of the return value. Related, but not exactly the same, see commit b784c7707502 ("coccinnelle: Remove ptr_ret script"). Adding Cc's from that commit for input. BR, Jani. > Reported-by: Zeal Robot <zealci@zte.com.cn> > Signed-off-by: ye xingchen <ye.xingchen@zte.com.cn> > --- > drivers/gpu/drm/drm_gem.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index ad068865ba20..3fa0deff3014 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -782,14 +782,11 @@ drm_gem_close_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_priv) > { > struct drm_gem_close *args = data; > - int ret; > > if (!drm_core_check_feature(dev, DRIVER_GEM)) > return -EOPNOTSUPP; > > - ret = drm_gem_handle_delete(file_priv, args->handle); > - > - return ret; > + return drm_gem_handle_delete(file_priv, args->handle); > } > > /** -- Jani Nikula, Intel Open Source Graphics Center ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH linux-next] drm/gem: Remove the unneeded result variable 2022-08-31 9:25 ` Jani Nikula @ 2022-08-31 12:02 ` Thierry Reding -1 siblings, 0 replies; 6+ messages in thread From: Thierry Reding @ 2022-08-31 12:02 UTC (permalink / raw) To: Jani Nikula Cc: airlied, cgel.zte, Zeal Robot, linux-kernel, dri-devel, Julia Lawall, Wolfram Sang, Mark Brown, Maxime Ripard, tzimmermann, ye xingchen [-- Attachment #1: Type: text/plain, Size: 2699 bytes --] On Wed, Aug 31, 2022 at 12:25:52PM +0300, Jani Nikula wrote: > On Wed, 31 Aug 2022, cgel.zte@gmail.com wrote: > > From: ye xingchen <ye.xingchen@zte.com.cn> > > > > Return the value drm_gem_handle_delete() directly instead of storing it > > in another redundant variable. > > Personally, I generally prefer keeping these types of variables, because > they make future changes on the code easier. For example, adding a new > call where you need to check the return value, or having to deal with > the error value right there. Or just adding ad hoc debug logging of the > return value. > > Related, but not exactly the same, see commit b784c7707502 > ("coccinnelle: Remove ptr_ret script"). Adding Cc's from that commit for > input. > > BR, > Jani. > > > Reported-by: Zeal Robot <zealci@zte.com.cn> > > Signed-off-by: ye xingchen <ye.xingchen@zte.com.cn> > > --- > > drivers/gpu/drm/drm_gem.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > index ad068865ba20..3fa0deff3014 100644 > > --- a/drivers/gpu/drm/drm_gem.c > > +++ b/drivers/gpu/drm/drm_gem.c > > @@ -782,14 +782,11 @@ drm_gem_close_ioctl(struct drm_device *dev, void *data, > > struct drm_file *file_priv) > > { > > struct drm_gem_close *args = data; > > - int ret; > > > > if (!drm_core_check_feature(dev, DRIVER_GEM)) > > return -EOPNOTSUPP; > > > > - ret = drm_gem_handle_delete(file_priv, args->handle); > > - > > - return ret; > > + return drm_gem_handle_delete(file_priv, args->handle); > > } > > > > /** I agree with Jani here. If I recall correctly, the pattern that the coccinelle script was dealing with was slightly different and in this example would've looked like this. ret = drm_gem_handle_delete(file_priv, args->handle); if (ret) return ret; return 0; You can collapse that in the same way, but I think it makes it even clearer than the original why it makes sense to keep this construct. If you even need to add any other cleanup code between the "return ret;" and "return 0;" lines, you can do so without rearranging the rest of the code. It's obviously highly subjective which one is preferable, which is why the recommendation at the time was not to provide a script that would make any suggestions as to the "right" style. In my opinion, if you write the code in the condensed form, that's fine, but if you prefer the more verbose variant, that's equally fine. Trying to "fix" this up after the fact for no other reason than to save a few lines and/or a variable, however, is a complete waste of time. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH linux-next] drm/gem: Remove the unneeded result variable @ 2022-08-31 12:02 ` Thierry Reding 0 siblings, 0 replies; 6+ messages in thread From: Thierry Reding @ 2022-08-31 12:02 UTC (permalink / raw) To: Jani Nikula Cc: cgel.zte, airlied, tzimmermann, ye xingchen, Zeal Robot, linux-kernel, dri-devel, Maxime Ripard, Julia Lawall, Wolfram Sang, Mark Brown [-- Attachment #1: Type: text/plain, Size: 2699 bytes --] On Wed, Aug 31, 2022 at 12:25:52PM +0300, Jani Nikula wrote: > On Wed, 31 Aug 2022, cgel.zte@gmail.com wrote: > > From: ye xingchen <ye.xingchen@zte.com.cn> > > > > Return the value drm_gem_handle_delete() directly instead of storing it > > in another redundant variable. > > Personally, I generally prefer keeping these types of variables, because > they make future changes on the code easier. For example, adding a new > call where you need to check the return value, or having to deal with > the error value right there. Or just adding ad hoc debug logging of the > return value. > > Related, but not exactly the same, see commit b784c7707502 > ("coccinnelle: Remove ptr_ret script"). Adding Cc's from that commit for > input. > > BR, > Jani. > > > Reported-by: Zeal Robot <zealci@zte.com.cn> > > Signed-off-by: ye xingchen <ye.xingchen@zte.com.cn> > > --- > > drivers/gpu/drm/drm_gem.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > > index ad068865ba20..3fa0deff3014 100644 > > --- a/drivers/gpu/drm/drm_gem.c > > +++ b/drivers/gpu/drm/drm_gem.c > > @@ -782,14 +782,11 @@ drm_gem_close_ioctl(struct drm_device *dev, void *data, > > struct drm_file *file_priv) > > { > > struct drm_gem_close *args = data; > > - int ret; > > > > if (!drm_core_check_feature(dev, DRIVER_GEM)) > > return -EOPNOTSUPP; > > > > - ret = drm_gem_handle_delete(file_priv, args->handle); > > - > > - return ret; > > + return drm_gem_handle_delete(file_priv, args->handle); > > } > > > > /** I agree with Jani here. If I recall correctly, the pattern that the coccinelle script was dealing with was slightly different and in this example would've looked like this. ret = drm_gem_handle_delete(file_priv, args->handle); if (ret) return ret; return 0; You can collapse that in the same way, but I think it makes it even clearer than the original why it makes sense to keep this construct. If you even need to add any other cleanup code between the "return ret;" and "return 0;" lines, you can do so without rearranging the rest of the code. It's obviously highly subjective which one is preferable, which is why the recommendation at the time was not to provide a script that would make any suggestions as to the "right" style. In my opinion, if you write the code in the condensed form, that's fine, but if you prefer the more verbose variant, that's equally fine. Trying to "fix" this up after the fact for no other reason than to save a few lines and/or a variable, however, is a complete waste of time. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-31 12:03 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-08-31 3:33 [PATCH linux-next] drm/gem: Remove the unneeded result variable cgel.zte 2022-08-31 3:33 ` cgel.zte 2022-08-31 9:25 ` Jani Nikula 2022-08-31 9:25 ` Jani Nikula 2022-08-31 12:02 ` Thierry Reding 2022-08-31 12:02 ` Thierry Reding
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.