diff mbox series

[-next,v2] drm/imagination: Use memdup_user() helper

Message ID 20240902023300.1214753-1-ruanjinjie@huawei.com (mailing list archive)
State New, archived
Headers show
Series [-next,v2] drm/imagination: Use memdup_user() helper | expand

Commit Message

Jinjie Ruan Sept. 2, 2024, 2:33 a.m. UTC
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(-)

Comments

Frank Binns Sept. 2, 2024, 7:59 a.m. UTC | #1
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;
Matt Coster Sept. 2, 2024, 9:21 a.m. UTC | #2
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
Markus Elfring Sept. 2, 2024, 4:09 p.m. UTC | #3
> > 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
Matt Coster Sept. 3, 2024, 9:12 a.m. UTC | #4
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
Markus Elfring Sept. 3, 2024, 9:40 a.m. UTC | #5
>>>> 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
diff mbox series

Patch

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;