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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Jan 31, 2019 at 08:13:27AM -0700, Jan Beulich wrote:
> >>> On 31.01.19 at 15:49, <roger.pau@xxxxxxxxxx> wrote:
> > On Wed, Jan 30, 2019 at 08:28:09PM -0800, Christopher Clark wrote:
> >> +static struct argo_ring_info *
> >> +find_ring_info(const struct domain *d, const struct argo_ring_id *id)
> >> +{
> >> +    struct argo_ring_info *ring_info;
> >> +    const struct list_head *bucket;
> >> +
> >> +    ASSERT(LOCKING_Read_rings_L2(d));
> >> +
> >> +    /* List is not modified here. Search and return the match if found. */
> >> +    bucket = &d->argo->ring_hash[hash_index(id)];
> >> +
> >> +    list_for_each_entry(ring_info, bucket, node)
> > 
> > I'm not sure what's the policy regarding list_ macros, should spaces
> > be added between the parentheses?
> > 
> > list_for_each_entry ( ring_info, bucket, node )
> > 
> > I don't have a strong opinion either way.
> 
> We use a mix, because it's not really clear whether to consider such
> "kind-of-keywords".
> 
> >> +static void
> >> +pending_remove_all(const struct domain *d, struct argo_ring_info 
> >> *ring_info)
> >> +{
> >> +    struct pending_ent *ent, *next;
> >> +
> >> +    ASSERT(LOCKING_L3(d, ring_info));
> >> +
> >> +    /* Delete all pending notifications from this ring's list. */
> >> +    list_for_each_entry_safe(ent, next, &ring_info->pending, node)
> > 
> > list_first_entry_or_null and you can get rid of next.
> 
> Well, that's a substitution I wouldn't like to do while committing,
> as it means doing a little more than a simple text replacement.

From a functionality PoV the code does exactly the same, I just think
list_first_entry_or_null is more suitable and I haven't been convinced
by the argument posted on v5.

However since this is argos specific code I don't have such a strong
opinion, and it can always be improved later by a followup patch. My
RB stands despite the usage of list_for_each_entry_safe.

Roger.

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