| Message ID | 20231007064019.17472-1-likexu@tencent.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86/xsave: Remove 'return void' expression for 'void function' | expand |
У сб, 2023-10-07 у 14:40 +0800, Like Xu пише: > From: Like Xu <likexu@tencent.com> > > The requested info will be stored in 'guest_xsave->region' referenced by > the incoming pointer "struct kvm_xsave *guest_xsave", thus there is no need > to explicitly use return void expression for a void function "static void > kvm_vcpu_ioctl_x86_get_xsave(...)". The issue is caught with [-Wpedantic]. > > Fixes: 2d287ec65e79 ("x86/fpu: Allow caller to constrain xfeatures when copying to uabi buffer") > Signed-off-by: Like Xu <likexu@tencent.com> > --- > arch/x86/kvm/x86.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index fdb2b0e61c43..2571466a317f 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5503,8 +5503,8 @@ static void kvm_vcpu_ioctl_x86_get_xsave2(struct kvm_vcpu *vcpu, > static void kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu, > struct kvm_xsave *guest_xsave) > { > - return kvm_vcpu_ioctl_x86_get_xsave2(vcpu, (void *)guest_xsave->region, > - sizeof(guest_xsave->region)); > + kvm_vcpu_ioctl_x86_get_xsave2(vcpu, (void *)guest_xsave->region, > + sizeof(guest_xsave->region)); > } > > static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu, > > base-commit: 86701e115030e020a052216baa942e8547e0b487 Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> Best regards, Maxim Levitsky
Emm, did we miss this little fix ? On 11/10/2023 12:12 am, Maxim Levitsky wrote: > У сб, 2023-10-07 у 14:40 +0800, Like Xu пише: >> From: Like Xu <likexu@tencent.com> >> >> The requested info will be stored in 'guest_xsave->region' referenced by >> the incoming pointer "struct kvm_xsave *guest_xsave", thus there is no need >> to explicitly use return void expression for a void function "static void >> kvm_vcpu_ioctl_x86_get_xsave(...)". The issue is caught with [-Wpedantic]. >> >> Fixes: 2d287ec65e79 ("x86/fpu: Allow caller to constrain xfeatures when copying to uabi buffer") >> Signed-off-by: Like Xu <likexu@tencent.com> >> --- >> arch/x86/kvm/x86.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index fdb2b0e61c43..2571466a317f 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -5503,8 +5503,8 @@ static void kvm_vcpu_ioctl_x86_get_xsave2(struct kvm_vcpu *vcpu, >> static void kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu, >> struct kvm_xsave *guest_xsave) >> { >> - return kvm_vcpu_ioctl_x86_get_xsave2(vcpu, (void *)guest_xsave->region, >> - sizeof(guest_xsave->region)); >> + kvm_vcpu_ioctl_x86_get_xsave2(vcpu, (void *)guest_xsave->region, >> + sizeof(guest_xsave->region)); >> } >> >> static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu, >> >> base-commit: 86701e115030e020a052216baa942e8547e0b487 > Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> > > Best regards, > Maxim Levitsky >
On Wed, Oct 25, 2023, Like Xu wrote:
> Emm, did we miss this little fix ?
No, I have it earmarked, it's just not a priority because it doesn't truly fix
anything. Though I suppose it probably makes to apply it for 6.8, waiting one
more day to send PULL requests to Paolo isn't a problem.
On Wed, Oct 25, 2023, Sean Christopherson wrote: > On Wed, Oct 25, 2023, Like Xu wrote: > > Emm, did we miss this little fix ? > > No, I have it earmarked, it's just not a priority because it doesn't truly fix > anything. Though I suppose it probably makes to apply it for 6.8, waiting one > more day to send PULL requests to Paolo isn't a problem. Heh, when I tried to apply this I got reminded of why I held it for later. I want to apply it to kvm-x86/misc, but that's based on ~6.6-rc2 (plus a few KVM patches), i.e. doesn't have the "buggy" commit. I don't want to rebase "misc", nor do I want to create a branch and PULL request for a single trivial commit. So for logistical reasons, I'm not going apply this right away, but I will make sure it gets into v6.7.
On 26/10/2023 9:10 am, Sean Christopherson wrote: > On Wed, Oct 25, 2023, Sean Christopherson wrote: >> On Wed, Oct 25, 2023, Like Xu wrote: >>> Emm, did we miss this little fix ? >> >> No, I have it earmarked, it's just not a priority because it doesn't truly fix >> anything. Though I suppose it probably makes to apply it for 6.8, waiting one >> more day to send PULL requests to Paolo isn't a problem. > > Heh, when I tried to apply this I got reminded of why I held it for later. I > want to apply it to kvm-x86/misc, but that's based on ~6.6-rc2 (plus a few KVM > patches), i.e. doesn't have the "buggy" commit. I don't want to rebase "misc", > nor do I want to create a branch and PULL request for a single trivial commit. > > So for logistical reasons, I'm not going apply this right away, but I will make > sure it gets into v6.7. Thanks, and a similar pattern occurs with these functions: 'write_register_operand' 'account_shadowed' 'unaccount_shadowed' 'mtrr_lookup_fixed_next' 'pre_svm_run' 'svm_vcpu_deliver_sipi_vector' Although the compiler will do the right thing, use 'return void' expression deliberately without grounds for exemption may annoy some CI pipelines. If you need more cleanup or a new version to cover all these cases above, just let me know.
On Thu, Nov 30, 2023, Like Xu wrote: > On 26/10/2023 9:10 am, Sean Christopherson wrote: > > On Wed, Oct 25, 2023, Sean Christopherson wrote: > > > On Wed, Oct 25, 2023, Like Xu wrote: > > > > Emm, did we miss this little fix ? > > > > > > No, I have it earmarked, it's just not a priority because it doesn't truly fix > > > anything. Though I suppose it probably makes to apply it for 6.8, waiting one > > > more day to send PULL requests to Paolo isn't a problem. > > > > Heh, when I tried to apply this I got reminded of why I held it for later. I > > want to apply it to kvm-x86/misc, but that's based on ~6.6-rc2 (plus a few KVM > > patches), i.e. doesn't have the "buggy" commit. I don't want to rebase "misc", > > nor do I want to create a branch and PULL request for a single trivial commit. > > > > So for logistical reasons, I'm not going apply this right away, but I will make > > sure it gets into v6.7. > > Thanks, and a similar pattern occurs with these functions: > > 'write_register_operand' > 'account_shadowed' > 'unaccount_shadowed' > 'mtrr_lookup_fixed_next' > 'pre_svm_run' > 'svm_vcpu_deliver_sipi_vector' > > Although the compiler will do the right thing, use 'return void' expression > deliberately without grounds for exemption may annoy some CI pipelines. > > If you need more cleanup or a new version to cover all these cases above, > just let me know. I'd rather update the CI pipelines to turn off -Wpedantic. There is zero chance that -Wpedantic will ever get enabled for kernel builds, the kernel is deliberately not ISO C compliant. I have no objection to cleaning up kvm_vcpu_ioctl_x86_get_xsave() because it's an obvious goof and a recent change, but like checkpatch warnings, I don't want to go around "fixing" warnings unless they are actively problematic for humans.
On Sat, 07 Oct 2023 14:40:19 +0800, Like Xu wrote: > The requested info will be stored in 'guest_xsave->region' referenced by > the incoming pointer "struct kvm_xsave *guest_xsave", thus there is no need > to explicitly use return void expression for a void function "static void > kvm_vcpu_ioctl_x86_get_xsave(...)". The issue is caught with [-Wpedantic]. > > Applied to kvm-x86 fixes, thanks! [1/1] KVM: x86/xsave: Remove 'return void' expression for 'void function' https://github.com/kvm-x86/linux/commit/ef8d89033c3f -- https://github.com/kvm-x86/linux/tree/next
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index fdb2b0e61c43..2571466a317f 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5503,8 +5503,8 @@ static void kvm_vcpu_ioctl_x86_get_xsave2(struct kvm_vcpu *vcpu, static void kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu, struct kvm_xsave *guest_xsave) { - return kvm_vcpu_ioctl_x86_get_xsave2(vcpu, (void *)guest_xsave->region, - sizeof(guest_xsave->region)); + kvm_vcpu_ioctl_x86_get_xsave2(vcpu, (void *)guest_xsave->region, + sizeof(guest_xsave->region)); } static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu,