Hi Oleg,

Sorry for my late response!

On 11/16/2013 01:12 AM, Oleg Nesterov wrote:

> On 11/15, Oleg Nesterov wrote:
>>
>> But probably list_last_entry() makes sense in the "else" branch,
>> athough this is minor.
>>
>>
>> Off-topic. Not sure this really makes sense, but I was thinking about
>>
>>      list_get_first(pos, head, member)       \
>>              ((pos) = list_first_entry(head, typeof(*pos), member))
>>
>> and list_get_first() last of course. The obvious advantage is that
>> compared to
>>
>>      tr = list_last_entry(sdp->sd_ail1_list, struct gfs2_trans, tr_list);
>>
>> above you do not need to type "struct gfs2_trans",
>>
>>      list_get_last(tr, sdp->sd_ail1_list, tr_list);
>>
>> looks a bit better.
> 
> And list.h can use it too.
> 
> So how about the patch below? I can't decide whether it cleanups or
> uglfies the code, I do not trust my taste ;) But to me it looks a
> bit better this way.

I personally think this is better for GFS2 case if I understood correctly,
and it can make the existing list helper a bit neat per my taste :)

The current list_first_entry() works fine consider a common pattern below:
while (!list_empty(ptr)) {
        struct dummy *p = list_first_entry(ptr, struct dummy, member);

        ....
}

But I'm not sure if we can find out more external use cases can get
benefits with list_get_first/last...

Thanks,
-Jeff

> 
> Oleg.
> --
> 
> diff --git a/include/linux/list.h b/include/linux/list.h
> index ef95941..f52aba8 100644
> --- a/include/linux/list.h
> +++ b/include/linux/list.h
> @@ -372,6 +372,12 @@ static inline void list_splice_tail_init(struct 
> list_head *list,
>  #define list_last_entry(ptr, type, member) \
>       list_entry((ptr)->prev, type, member)
>  
> +#define list_get_first(pos, head, member)    \
> +     ((pos) = list_first_entry(head, typeof(*(pos)), member))
> +
> +#define list_get_last(pos, head, member)     \
> +     ((pos) = list_last_entry(head, typeof(*(pos)), member))
> +
>  /**
>   * list_first_entry_or_null - get the first element from a list
>   * @ptr:     the list head to take the element from.
> @@ -443,7 +449,7 @@ static inline void list_splice_tail_init(struct list_head 
> *list,
>   * @member:  the name of the list_struct within the struct.
>   */
>  #define list_for_each_entry(pos, head, member)                               
> \
> -     for (pos = list_first_entry(head, typeof(*pos), member);        \
> +     for (list_get_first(pos, head, member);                         \
>            &pos->member != (head);                                    \
>            pos = list_next_entry(pos, member))
>  
> @@ -454,7 +460,7 @@ static inline void list_splice_tail_init(struct list_head 
> *list,
>   * @member:  the name of the list_struct within the struct.
>   */
>  #define list_for_each_entry_reverse(pos, head, member)                       
> \
> -     for (pos = list_last_entry(head, typeof(*pos), member);         \
> +     for (list_get_last(pos, head, member);                          \
>            &pos->member != (head);                                    \
>            pos = list_prev_entry(pos, member))
>  
> @@ -517,7 +523,7 @@ static inline void list_splice_tail_init(struct list_head 
> *list,
>   * @member:  the name of the list_struct within the struct.
>   */
>  #define list_for_each_entry_safe(pos, n, head, member)                       
> \
> -     for (pos = list_first_entry(head, typeof(*pos), member),        \
> +     for (list_get_first(pos, head, member),                         \
>               n = list_next_entry(pos, member);                       \
>            &pos->member != (head);                                    \
>            pos = n, n = list_next_entry(n, member))
> @@ -564,7 +570,7 @@ static inline void list_splice_tail_init(struct list_head 
> *list,
>   * of list entry.
>   */
>  #define list_for_each_entry_safe_reverse(pos, n, head, member)               
> \
> -     for (pos = list_last_entry(head, typeof(*pos), member),         \
> +     for (list_get_last(pos, head, member),                          \
>               n = list_prev_entry(pos, member);                       \
>            &pos->member != (head);                                    \
>            pos = n, n = list_prev_entry(n, member))
> 
> _______________________________________________
> xfs mailing list
> [email protected]
> http://oss.sgi.com/mailman/listinfo/xfs



------------------------------------------------------------------------------
DreamFactory - Open Source REST & JSON Services for HTML5 & Native Apps
OAuth, Users, Roles, SQL, NoSQL, BLOB Storage and External API Access
Free app hosting. Or install the open source package on any LAMP server.
Sign up and see examples for AngularJS, jQuery, Sencha Touch and Native!
http://pubads.g.doubleclick.net/gampad/clk?id=63469471&iu=/4140/ostg.clktrk
_______________________________________________
Jfs-discussion mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/jfs-discussion

Reply via email to