Re: [Xen-devel] [PATCH v5 04/15] argo: init, destroy and soft-reset, with enable command line opt

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.01.19 at 05:06, <christopher.w.clark@xxxxxxxxx> wrote:
> On Mon, Jan 21, 2019 at 9:55 AM Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
>> On Mon, Jan 21, 2019 at 01:59:44AM -0800, Christopher Clark wrote:
>> > +static void
>> > +pending_remove_all(const struct domain *d, struct argo_ring_info 
>> > *ring_info)
>> > +{
>> > +    struct list_head *ring_pending = &ring_info->pending;
>> > +    struct pending_ent *ent;
>> > +
>> > +    ASSERT(LOCKING_L3(d, ring_info));
>> > +
>> > +    /* Delete all pending notifications from this ring's list. */
>> > +    while ( !list_empty(ring_pending) )
>>
>> Nit: you could use list_first_entry_or_null that joins the list_empty
>> and list_entry calls.
> 
> There are no existing users of list_first_entry_or_null anywhere in Xen,
> and applying it to this loop results in an assignment within the
> while condition, which also appears to be very rare construct within Xen,
> so I just used the list_for_each_entry_safe macro.

I'm not fully following why lack of use of a construct elsewhere in the
tree would be a reason not to use it here. If you were to use only
constructs already in use in common code, you should also not have
had a need to e.g. introduce for Arm (and use) guest_handle_for_field().

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel