Re: [PATCH v1 3/8] s390/sclp: rework sclp boundary and length checks

[Cornelia Huck]

On Tue, 12 May 2020 10:55:56 -0400
Collin Walling <address@hidden> 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;
> >> +}
> >> +