From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50606) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fXTlF-0008Hr-H1 for qemu-devel@nongnu.org; Mon, 25 Jun 2018 11:50:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fXTlB-0000lK-JO for qemu-devel@nongnu.org; Mon, 25 Jun 2018 11:50:37 -0400 Date: Mon, 25 Jun 2018 17:50:29 +0200 From: Cornelia Huck Message-ID: <20180625175029.69643252.cohuck@redhat.com> In-Reply-To: <20180625115352.6889-3-david@redhat.com> References: <20180625115352.6889-1-david@redhat.com> <20180625115352.6889-3-david@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 2/9] s390x/kvm: pass values instead of pointers to kvm_s390_set_clock_*() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Hildenbrand Cc: qemu-s390x@nongnu.org, qemu-devel@nongnu.org, Richard Henderson , Alexander Graf , Christian Borntraeger , Thomas Huth On Mon, 25 Jun 2018 13:53:45 +0200 David Hildenbrand 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 > --- > 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 */