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.
>
> >
> >> +
> >> + 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;
> >> +}
> >> +
