Re: [Qemu-devel] [PATCH v3 2/9] s390x/kvm: pass values instead of pointers to kvm_s390_set_clock_*()

[Cornelia Huck <cohuck@redhat.com>]

On Mon, 25 Jun 2018 13:53:45 +0200
David Hildenbrand <david@redhat.com> wrote:

> We are going to factor out the TOD into a separate device and use const
> pointers for device class functions where possible. We are passing right
> now ordinary pointers that should never be touched when setting the TOD.
> Let's just pass the values directly.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/cpu.c       |  4 ++--
>  target/s390x/kvm-stub.c  |  4 ++--
>  target/s390x/kvm.c       | 12 ++++++------
>  target/s390x/kvm_s390x.h |  4 ++--
>  4 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index c268065887..68512e3e54 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -413,9 +413,9 @@ int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low)

Any reason why you keep the pointers here?

>      int r = 0;
>  
>      if (kvm_enabled()) {
> -        r = kvm_s390_set_clock_ext(tod_high, tod_low);
> +        r = kvm_s390_set_clock_ext(*tod_high, *tod_low);
>          if (r == -ENXIO) {
> -            return kvm_s390_set_clock(tod_high, tod_low);
> +            return kvm_s390_set_clock(*tod_high, *tod_low);

Especially as it would be more clean to check for !NULL before
dereferencing...

>          }
>      }
>      /* Fixme TCG */