On 11/2/22 11:33, Thomas Zimmermann wrote:
[...]
>>
>>> +static ssize_t __drm_fb_helper_write(struct fb_info *info, const char __user *buf, size_t count,
>>> + loff_t *ppos, drm_fb_helper_write_screen write_screen)
>>> +{
>>
>> [...]
>>
>>> + /*
>>> + * Copy to framebuffer even if we already logged an error. Emulates
>>> + * the behavior of the original fbdev implementation.
>>> + */
>>> + ret = write_screen(info, buf, count, pos);
>>> + if (ret < 0)
>>> + return ret; /* return last error, if any */
>>> + else if (!ret)
>>> + return err; /* return previous error, if any */
>>> +
>>> + *ppos += ret;
>>> +
>>
>> Should *ppos be incremented even if the previous error is returned?
>
> Yes. It emulates the original fbdev code at [1]. Further down in that
> function, the position is being updated even if an error occured. We
> only return the initial error if no bytes got written.
>
> It could happen that some userspace program hits to error, but still
> relies on the output and position being updated. IIRC I even added
> validation of this behavior to the IGT fbdev tests. I agree that this
> is somewhat bogus behavior, but changing it would change long-standing
> userspace semantics.
>
Thanks for the explanation, feel free then to also add to this patch:
Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
