Re: [Xen-devel] [PATCH] grant-table, xen-netback: Introduce helper functions for grant copy operations

[David Vrabel <david.vrabel@xxxxxxxxxx>]

On 03/04/14 09:12, Paul Durrant wrote:
> Zoltan Kiss wrote:
>>
>> Create helper functions for grant copy operations and use them in netback.
>>
[...]
>> --- a/drivers/net/xen-netback/netback.c
>> +++ b/drivers/net/xen-netback/netback.c
>> @@ -275,23 +275,29 @@ static void xenvif_gop_frag_copy(struct xenvif *vif,
>> struct sk_buff *skb,
>>                      bytes = MAX_BUFFER_OFFSET - npo->copy_off;
>>
>>              copy_gop = npo->copy + npo->copy_prod++;
>> -            copy_gop->flags = GNTCOPY_dest_gref;
>> -            copy_gop->len = bytes;
>>
>>              if (foreign_vif) {
>> -                    copy_gop->source.domid = foreign_vif->domid;
>> -                    copy_gop->source.u.ref = foreign_gref;
>> -                    copy_gop->flags |= GNTCOPY_source_gref;
>> +                    gnttab_set_copy_op_ref_to_ref(copy_gop,
>> +                                                  foreign_gref,
>> +                                                  npo->copy_gref,
>> +                                                  foreign_vif->domid,
>> +                                                  offset,
>> +                                                  vif->domid,
>> +                                                  npo->copy_off,
>> +                                                  bytes,
>> +                                                  GNTCOPY_dest_gref |
>> +                                                  GNTCOPY_source_gref);
> 
> Can't you lose the flags here (and in the other variants)? Since
> they are implied by the variant of the function they could just be hardcoded
> there, couldn't they?

Even with that change these are still functions with 7 or 8 parameters
with no obvious order.  That's just awful.  The open-coded struct
initialization is better.

David

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