Re: [patch V3 59/66] x86/fpu/signal: Move initial checks into fpu__sig_restore()

[Thomas Gleixner]

On Tue, Jun 22 2021 at 19:35, Borislav Petkov wrote:
> On Fri, Jun 18, 2021 at 04:19:22PM +0200, Thomas Gleixner wrote:
>>  
>> +	if (unlikely(!buf)) {
>> +		fpu__clear_user_states(&current->thread.fpu);
>
> You could declare
>
> 	struct fpu *fpu = &tsk->thread.fpu;
>
> above so that it is easier to read, as this call is done twice.
>
> Also, you can do:
>
> 	int ret = 0;
>
> 	if (unlikely(!buf))
> 		goto out;
>
> so that the exit paths converge at the end.

I pondered, but look at the condition there. It gets unreadable.

So I kept is as is because this is an intentional clear which returns
success and the other is on error.

>> +	if (!IS_ENABLED(CONFIG_X86_64) && !static_cpu_has(X86_FEATURE_FPU)) {
>
> cpu_feature_enabled()
>
>> +		return fpregs_soft_set(current, NULL, 0,
>> +				       sizeof(struct user_i387_ia32_struct),
>> +				       NULL, buf);
>
> Err, don't you need to catch retval into ret here and goto out, like
> before, so that you can call fpu__clear_user_states() on error?

Yes. Actually we should do that as a separate patch way earlier in the
series. Sigh.