public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/xe/query: Refactor copy_to_user() usage in four functions
@ 2024-09-18  8:12 Markus Elfring
  2024-09-18  9:29 ` Jani Nikula
  0 siblings, 1 reply; 3+ messages in thread
From: Markus Elfring @ 2024-09-18  8:12 UTC (permalink / raw)
  To: intel-xe, dri-devel, kernel-janitors, Daniel Vetter, David Airlie,
	Faith Ekstrand, Francois Dugast, Jani Nikula,
	José Roberto de Souza, Lucas De Marchi, Maarten Lankhorst,
	Matt Roper, Matthew Brost, Mauro Carvalho Chehab, Maxime Ripard,
	Nirmoy Das, Philippe Lecluse, Rodrigo Vivi, Simona Vetter,
	Thomas Hellström, Thomas Zimmermann
  Cc: LKML

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 18 Sep 2024 09:43:07 +0200

Assign return values from copy_to_user() calls to additional local variables
so that four kfree() calls and return statements can be omitted accordingly.

This issue was transformed by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/xe/xe_query.c | 32 ++++++++++++--------------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
index 5246a4a2740e..6195e720176d 100644
--- a/drivers/gpu/drm/xe/xe_query.c
+++ b/drivers/gpu/drm/xe/xe_query.c
@@ -220,13 +220,11 @@ static int query_engines(struct xe_device *xe,

 	engines->num_engines = i;

-	if (copy_to_user(query_ptr, engines, size)) {
+	{
+		unsigned long ctu = copy_to_user(query_ptr, engines, size);
 		kfree(engines);
-		return -EFAULT;
+		return ctu ? -EFAULT : 0;
 	}
-	kfree(engines);
-
-	return 0;
 }

 static size_t calc_mem_regions_size(struct xe_device *xe)
@@ -344,13 +342,11 @@ static int query_config(struct xe_device *xe, struct drm_xe_device_query *query)
 	config->info[DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY] =
 		xe_exec_queue_device_get_max_priority(xe);

-	if (copy_to_user(query_ptr, config, size)) {
+	{
+		unsigned long ctu = copy_to_user(query_ptr, config, size);
 		kfree(config);
-		return -EFAULT;
+		return ctu ? -EFAULT : 0;
 	}
-	kfree(config);
-
-	return 0;
 }

 static int query_gt_list(struct xe_device *xe, struct drm_xe_device_query *query)
@@ -414,13 +410,11 @@ static int query_gt_list(struct xe_device *xe, struct drm_xe_device_query *query
 			REG_FIELD_GET(GMD_ID_REVID, gt->info.gmdid);
 	}

-	if (copy_to_user(query_ptr, gt_list, size)) {
+	{
+		unsigned long ctu = copy_to_user(query_ptr, gt_list, size);
 		kfree(gt_list);
-		return -EFAULT;
+		return ctu ? -EFAULT : 0;
 	}
-	kfree(gt_list);
-
-	return 0;
 }

 static int query_hwconfig(struct xe_device *xe,
@@ -444,13 +438,11 @@ static int query_hwconfig(struct xe_device *xe,

 	xe_guc_hwconfig_copy(&gt->uc.guc, hwconfig);

-	if (copy_to_user(query_ptr, hwconfig, size)) {
+	{
+		unsigned long ctu = copy_to_user(query_ptr, hwconfig, size);
 		kfree(hwconfig);
-		return -EFAULT;
+		return ctu ? -EFAULT : 0;
 	}
-	kfree(hwconfig);
-
-	return 0;
 }

 static size_t calc_topo_query_size(struct xe_device *xe)
--
2.46.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/xe/query: Refactor copy_to_user() usage in four functions
  2024-09-18  8:12 [PATCH] drm/xe/query: Refactor copy_to_user() usage in four functions Markus Elfring
@ 2024-09-18  9:29 ` Jani Nikula
  2024-09-18  9:38   ` Markus Elfring
  0 siblings, 1 reply; 3+ messages in thread
From: Jani Nikula @ 2024-09-18  9:29 UTC (permalink / raw)
  To: Markus Elfring, intel-xe, dri-devel, kernel-janitors,
	Daniel Vetter, David Airlie, Faith Ekstrand, Francois Dugast,
	José Roberto de Souza, Lucas De Marchi, Maarten Lankhorst,
	Matt Roper, Matthew Brost, Mauro Carvalho Chehab, Maxime Ripard,
	Nirmoy Das, Philippe Lecluse, Rodrigo Vivi, Simona Vetter,
	Thomas Hellström, Thomas Zimmermann
  Cc: LKML

On Wed, 18 Sep 2024, Markus Elfring <Markus.Elfring@web.de> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 18 Sep 2024 09:43:07 +0200
>
> Assign return values from copy_to_user() calls to additional local variables
> so that four kfree() calls and return statements can be omitted accordingly.
>
> This issue was transformed by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/gpu/drm/xe/xe_query.c | 32 ++++++++++++--------------------
>  1 file changed, 12 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> index 5246a4a2740e..6195e720176d 100644
> --- a/drivers/gpu/drm/xe/xe_query.c
> +++ b/drivers/gpu/drm/xe/xe_query.c
> @@ -220,13 +220,11 @@ static int query_engines(struct xe_device *xe,
>
>  	engines->num_engines = i;
>
> -	if (copy_to_user(query_ptr, engines, size)) {
> +	{

Please don't leave blocks like this behind when you remove the if.

BR,
Jani.

> +		unsigned long ctu = copy_to_user(query_ptr, engines, size);
>  		kfree(engines);
> -		return -EFAULT;
> +		return ctu ? -EFAULT : 0;
>  	}
> -	kfree(engines);
> -
> -	return 0;
>  }
>
>  static size_t calc_mem_regions_size(struct xe_device *xe)
> @@ -344,13 +342,11 @@ static int query_config(struct xe_device *xe, struct drm_xe_device_query *query)
>  	config->info[DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY] =
>  		xe_exec_queue_device_get_max_priority(xe);
>
> -	if (copy_to_user(query_ptr, config, size)) {
> +	{
> +		unsigned long ctu = copy_to_user(query_ptr, config, size);
>  		kfree(config);
> -		return -EFAULT;
> +		return ctu ? -EFAULT : 0;
>  	}
> -	kfree(config);
> -
> -	return 0;
>  }
>
>  static int query_gt_list(struct xe_device *xe, struct drm_xe_device_query *query)
> @@ -414,13 +410,11 @@ static int query_gt_list(struct xe_device *xe, struct drm_xe_device_query *query
>  			REG_FIELD_GET(GMD_ID_REVID, gt->info.gmdid);
>  	}
>
> -	if (copy_to_user(query_ptr, gt_list, size)) {
> +	{
> +		unsigned long ctu = copy_to_user(query_ptr, gt_list, size);
>  		kfree(gt_list);
> -		return -EFAULT;
> +		return ctu ? -EFAULT : 0;
>  	}
> -	kfree(gt_list);
> -
> -	return 0;
>  }
>
>  static int query_hwconfig(struct xe_device *xe,
> @@ -444,13 +438,11 @@ static int query_hwconfig(struct xe_device *xe,
>
>  	xe_guc_hwconfig_copy(&gt->uc.guc, hwconfig);
>
> -	if (copy_to_user(query_ptr, hwconfig, size)) {
> +	{
> +		unsigned long ctu = copy_to_user(query_ptr, hwconfig, size);
>  		kfree(hwconfig);
> -		return -EFAULT;
> +		return ctu ? -EFAULT : 0;
>  	}
> -	kfree(hwconfig);
> -
> -	return 0;
>  }
>
>  static size_t calc_topo_query_size(struct xe_device *xe)
> --
> 2.46.0
>

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/xe/query: Refactor copy_to_user() usage in four functions
  2024-09-18  9:29 ` Jani Nikula
@ 2024-09-18  9:38   ` Markus Elfring
  0 siblings, 0 replies; 3+ messages in thread
From: Markus Elfring @ 2024-09-18  9:38 UTC (permalink / raw)
  To: Jani Nikula, intel-xe, dri-devel, kernel-janitors, Daniel Vetter,
	David Airlie, Faith Ekstrand, Francois Dugast,
	José Roberto de Souza, Lucas De Marchi, Maarten Lankhorst,
	Matt Roper, Matthew Brost, Mauro Carvalho Chehab, Maxime Ripard,
	Nirmoy Das, Philippe Lecluse, Rodrigo Vivi, Simona Vetter,
	Thomas Hellström, Thomas Zimmermann
  Cc: LKML

>> Assign return values from copy_to_user() calls to additional local variables
>> so that four kfree() calls and return statements can be omitted accordingly.
…
>> +++ b/drivers/gpu/drm/xe/xe_query.c
>> @@ -220,13 +220,11 @@ static int query_engines(struct xe_device *xe,
>>
>>  	engines->num_engines = i;
>>
>> -	if (copy_to_user(query_ptr, engines, size)) {
>> +	{
>
> Please don't leave blocks like this behind when you remove the if.
…
>> +		unsigned long ctu = copy_to_user(query_ptr, engines, size);
>>  		kfree(engines);
>> -		return -EFAULT;
>> +		return ctu ? -EFAULT : 0;
>>  	}
>> -	kfree(engines);
>> -
>> -	return 0;
>>  }
…

Would you tolerate the shown variable definition without the proposed
compound statement?

Regards,
Markus

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-09-18  9:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-18  8:12 [PATCH] drm/xe/query: Refactor copy_to_user() usage in four functions Markus Elfring
2024-09-18  9:29 ` Jani Nikula
2024-09-18  9:38   ` Markus Elfring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox