Re: [Xen-devel] [PATCH RESEND v5 09/24] x86: refactor psr: set value: assemble features value array.

[Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>]

On 17-01-31 15:57:26, Konrad Rzeszutek Wilk wrote:
> >  static int assemble_val_array(uint64_t *val,
> > @@ -550,7 +641,25 @@ static int assemble_val_array(uint64_t *val,
> >                                const struct psr_socket_info *info,
> >                                unsigned int old_cos)
> >  {
> > -    return -EINVAL;
> > +    const struct feat_node *feat;
> > +    int ret;
> > +    uint64_t *val_tmp = val;
> > +
> > +    if ( !val )
> > +        return -EINVAL;
> > +
> > +    /* Get all features current values according to old_cos. */
> > +    list_for_each_entry(feat, &info->feat_list, list)
> > +    {
> > +        /* value getting order is same as feature list */
> > +        ret = feat->ops.get_old_val(val_tmp, feat, old_cos);
> 
> Shouldn't we check ret for negative values?
> > +
> > +        val_tmp += feat->ops.get_cos_num(feat);
> > +        if ( val_tmp - val > array_len)
> > +            return -EINVAL;
> 
> 
> Perhaps: ENOSPC ?
> 
> Also this function does do any assembling. It just verifies.
> Perhaps other patches add extra work here? In which case you may
> want to mention that in the commit description.
> 
Array "assembling" is done here. But maybe the "assembling" is not accurate.
In this function, I want to get all features old values and put them into
the array according to every feature's position in feature list.

Do you have more accurate word than "assemble"? Thanks!

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel