Switching to memdup_user(), which combines kmalloc() and copy_from_user(),
and it can simplfy code.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Suggested-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
v2:
- Add suggested-by.
- Simplify the code.
---
 drivers/gpu/drm/imagination/pvr_context.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_context.c b/drivers/gpu/drm/imagination/pvr_context.c
index eded5e955cc0..98327f9bbd9c 100644
--- a/drivers/gpu/drm/imagination/pvr_context.c
+++ b/drivers/gpu/drm/imagination/pvr_context.c
@@ -69,24 +69,12 @@ process_static_context_state(struct pvr_device *pvr_dev, const struct pvr_stream
 	void *stream;
 	int err;
 
-	stream = kzalloc(stream_size, GFP_KERNEL);
-	if (!stream)
-		return -ENOMEM;
-
-	if (copy_from_user(stream, u64_to_user_ptr(stream_user_ptr), stream_size)) {
-		err = -EFAULT;
-		goto err_free;
-	}
+	stream = memdup_user(u64_to_user_ptr(stream_user_ptr), stream_size);
+	if (IS_ERR(stream))
+		return PTR_ERR(stream);
 
 	err = pvr_stream_process(pvr_dev, cmd_defs, stream, stream_size, dest);
-	if (err)
-		goto err_free;
-
-	kfree(stream);
-
-	return 0;
 
-err_free:
 	kfree(stream);
 
 	return err;
-- 
2.34.1

On 02/09/2024 03:33, Jinjie Ruan wrote:
> Switching to memdup_user(), which combines kmalloc() and copy_from_user(),
> and it can simplfy code.

Applied, thanks!

[1/1] drm/imagination: Use memdup_user() helper
      commit: 2872a57c7ad427d428c6d12e95e55b32bdc8e3b8

Cheers,
Matt

> > Switching to memdup_user(), which combines kmalloc() and copy_from_user(),
> > and it can simplfy code.
>
> Applied, thanks!
>
> [1/1] drm/imagination: Use memdup_user() helper
>       commit: 2872a57c7ad427d428c6d12e95e55b32bdc8e3b8

Do you find any previous contributions still similarly interesting?

Example:
[PATCH] drm/imagination: Use memdup_user() rather than duplicating its implementation
https://lore.kernel.org/r/c07221ed-8eaf-490e-9672-033b1cfe7b6e@web.de
https://lkml.org/lkml/2024/1/28/438

Regards,
Markus

On 02/09/2024 17:09, Markus Elfring wrote:
>>> Switching to memdup_user(), which combines kmalloc() and copy_from_user(),
>>> and it can simplfy code.
>>
>> Applied, thanks!
>>
>> [1/1] drm/imagination: Use memdup_user() helper
>>       commit: 2872a57c7ad427d428c6d12e95e55b32bdc8e3b8
> 
> Do you find any previous contributions still similarly interesting?
> 
> Example:
> [PATCH] drm/imagination: Use memdup_user() rather than duplicating its implementation
> https://lore.kernel.org/r/c07221ed-8eaf-490e-9672-033b1cfe7b6e@web.de
> https://lkml.org/lkml/2024/1/28/438

Hi Markus,

I apologise for missing your earlier email. In general, we'll happily
accept cleanup patches.

If you feel like your patch has gone ignored in future, please feel free
to ping me directly either by email or on IRC at MTCoster.

Cheers,
Matt

---
Matt Coster
E: matt.coster@imgtec.com
 
> Regards,
> Markus

>>>> Switching to memdup_user(), which combines kmalloc() and copy_from_user(),
>>>> and it can simplfy code.

By the way:
Would it have been nicer to avoid a typo anyhow in such a change description?


>>> Applied, thanks!
>>>
>>> [1/1] drm/imagination: Use memdup_user() helper
>>>       commit: 2872a57c7ad427d428c6d12e95e55b32bdc8e3b8
>>
>> Do you find any previous contributions still similarly interesting?
>>
>> Example:
>> [PATCH] drm/imagination: Use memdup_user() rather than duplicating its implementation
>> https://lore.kernel.org/r/c07221ed-8eaf-490e-9672-033b1cfe7b6e@web.de
>> https://lkml.org/lkml/2024/1/28/438
>
> Hi Markus,
>
> I apologise for missing your earlier email.

How could this happen?


> In general, we'll happily accept cleanup patches.
>
> If you feel like your patch has gone ignored in future,

It seems that some of my development ideas occasionally trigger special communication challenges.


> please feel free to ping me directly either by email or on IRC at MTCoster.

Will the attention really grow accordingly?

Regards,
Markus

On Mon, 2024-09-02 at 10:33 +0800, Jinjie Ruan wrote:
> Switching to memdup_user(), which combines kmalloc() and copy_from_user(),
> and it can simplfy code.
> 

Reviewed-by: Frank Binns <frank.binns@imgtec.com>

> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Suggested-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> v2:
> - Add suggested-by.
> - Simplify the code.
> ---
>  drivers/gpu/drm/imagination/pvr_context.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_context.c b/drivers/gpu/drm/imagination/pvr_context.c
> index eded5e955cc0..98327f9bbd9c 100644
> --- a/drivers/gpu/drm/imagination/pvr_context.c
> +++ b/drivers/gpu/drm/imagination/pvr_context.c
> @@ -69,24 +69,12 @@ process_static_context_state(struct pvr_device *pvr_dev, const struct pvr_stream
>  	void *stream;
>  	int err;
>  
> -	stream = kzalloc(stream_size, GFP_KERNEL);
> -	if (!stream)
> -		return -ENOMEM;
> -
> -	if (copy_from_user(stream, u64_to_user_ptr(stream_user_ptr), stream_size)) {
> -		err = -EFAULT;
> -		goto err_free;
> -	}
> +	stream = memdup_user(u64_to_user_ptr(stream_user_ptr), stream_size);
> +	if (IS_ERR(stream))
> +		return PTR_ERR(stream);
>  
>  	err = pvr_stream_process(pvr_dev, cmd_defs, stream, stream_size, dest);
> -	if (err)
> -		goto err_free;
> -
> -	kfree(stream);
> -
> -	return 0;
>  
> -err_free:
>  	kfree(stream);
>  
>  	return err;