On 5/13/20 3:00 AM, Cornelia Huck wrote:
> On Tue, 12 May 2020 10:55:56 -0400
> Collin Walling <wall...@linux.ibm.com> wrote:
>
>> On 5/12/20 3:21 AM, David Hildenbrand wrote:
>>> On 09.05.20 01:08, Collin Walling wrote:
>
>>>> +static bool check_sufficient_sccb_len(SCCB *sccb, int size)
>>>
>>> "has_sufficient_sccb_len" ?
>>>
>>>> +{
>>>> + MachineState *ms = MACHINE(qdev_get_machine());
>>>> + int required_len = size + ms->possible_cpus->len * sizeof(CPUEntry);
>>>
>>> Rather pass in the number of cpus instead. Looking up the machine again
>>> in here is ugly.
>>
>> prepare_cpu_entries also looks up the machine again. Should I squeeze
>> in a cleanup where we pass the machine to that function too (perhaps
>> in the "remove SCLPDevice" patch)?
>
> sclp_read_cpu_info() does not have the machine handy, so you'd need to
> move machine lookup there; but I think it's worth getting rid of
> duplicate lookups.
>
Sounds good, then. I'll propose a change to patch 1 that removes the
unused SCLPDevice param and accepts a MachineState instead. Should make
things a bit cleaner :)
>>
>>>
>>>> +
>>>> + if (be16_to_cpu(sccb->h.length) < required_len) {
>>>> + sccb->h.response_code =
>>>> cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
>>>> + return false;
>>>> + }
>>>> + return true;
>>>> +}
>>>> +
>
>
--
--
Regards,
Collin
Stay safe and stay healthy
