Re: [PATCH 1/6] fbdev: Return number of bytes read or written

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 1785 bytes --]

Hi

Am 26.04.23 um 16:41 schrieb Geert Uytterhoeven:
> Hi Thomas,
> 
> On Tue, Apr 25, 2023 at 4:28 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Always return the number of bytes read or written within the
>> framebuffer. Only return an errno code if framebuffer memory
>> was not touched. This is the semantics required by POSIX and
>> makes fb_read() and fb_write() compatible with IGT tests. [1]
>>
>> This bug has been fixed for fb_write() long ago by
>> commit 6a2a88668e90 ("[PATCH] fbdev: Fix return error of
>> fb_write"). The code in fb_read() and the corresponding fb_sys_()
>> helpers was forgotten.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Link: https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/fbdev.c # 1
> 
> Thanks for your patch!
> 
>> --- a/drivers/video/fbdev/core/fbmem.c
>> +++ b/drivers/video/fbdev/core/fbmem.c
>> @@ -820,7 +820,7 @@ fb_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
>>
>>          kfree(buffer);
>>
>> -       return (err) ? err : cnt;
>> +       return cnt ? cnt : err;
>>   }
> 
> Looks all good to me, so
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> However, shouldn't the copy_to_user() handling in fb_read() be fixed,
> too?

That's a good point. It doesn't necessarily copy all given bytes and can 
thus return the wrong result. The IGT tests passed anyway, but I'll fix 
it in v2.

Best regards
Thomas

> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]