On 25.06.2018 18:03, Cornelia Huck wrote:
> On Mon, 25 Jun 2018 17:54:42 +0200
> David Hildenbrand <da...@redhat.com> wrote:
>
>> On 25.06.2018 17:50, Cornelia Huck wrote:
>>> On Mon, 25 Jun 2018 13:53:45 +0200
>>> David Hildenbrand <da...@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 <da...@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...
>>
>> See the next patch :)
>>
>> (I assume that refactoring code in order to rip it out does not make sense)
>
> Add a comment in the commit message?
>
> "Note that s390_set_clock() will be removed in a follow-on patch and
> therefore its calling convention is not changed."
>
Sure I can do that. Thanks!
--
Thanks,
David / dhildenb
