Re: [Mesa-dev] [PATCH 09/13] util/list: Add list_empty and list_length functions
Ian Romanick writes: >> For what it's worth, I'm strongly in favour of using these >> kernel-style lists instead of exec_list. The kernel ones seem much >> less confusing. > > Huh? They're practically identical. The only difference is the > kernel-style lists have a single sentinel node, and that node is > impossible to identify "in a crowd." The exec_lists use two sentinel > nodes, and those nodes have one pointer of overlapping storage (head > and tail are the next and prev pointers of one node, and tail and > tail_pred are the next and prev pointers of the other). I thought > there was some ASCII art in list.h that showed this, but that appears > to not be the case... Yes, I understand how they work. But you have to admit that the magic of making the end sentinel overlap with the head sentinel is a bit more difficult to get your head around then just having a single sentinel. At least personally I found that more confusing. > This gives some convenience that you can walk through a list from any > node in the list without having a pointer to the list itself. I don't > know if we still do, but there used to be a few places where we took > advantage of that. Ok, that is a good point. However if we can't find any examples of where we are doing this then maybe it isn't all that important. A counter advantage of the kernel-style lists is that the sentinel is slightly smaller (one fewer pointer). That might be an important consideration if you're using them to build up a tree structure with a lot of lists, like an AST. However the main advantage for me is that saying “they're easy, they're just like in the kernel and Wayland” is a lot more likely to have meaning for someone than saying “they're just like on the Amiga”! Regards, - Neil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/13] util/list: Add list_empty and list_length functions
On 05/05/2015 11:21 AM, Neil Roberts wrote: > Jason Ekstrand writes: > >> +static inline bool list_empty(struct list_head *list) >> +{ >> + return list->next == list; >> +} > > It would be good if list.h also included stdbool.h in order to get the > declaration of bool. However, will that cause problems on MSVC? Is the > Gallium code compiled on MSVC in general? > >> +static inline unsigned list_length(struct list_head *list) >> +{ >> + unsigned length = 0; >> + for (struct list_head *node = list->next; node != list; node = >> node->next) >> + length++; >> + return length; >> +} > > Any reason not to use one of the list iterator macros here? Is it safe > to use a C99-ism outside of a macro in this header? Maybe MSVC > supports this particular C99-ism anyway. > > For what it's worth, I'm strongly in favour of using these kernel-style > lists instead of exec_list. The kernel ones seem much less confusing. Huh? They're practically identical. The only difference is the kernel-style lists have a single sentinel node, and that node is impossible to identify "in a crowd." The exec_lists use two sentinel nodes, and those nodes have one pointer of overlapping storage (head and tail are the next and prev pointers of one node, and tail and tail_pred are the next and prev pointers of the other). I thought there was some ASCII art in list.h that showed this, but that appears to not be the case... This gives some convenience that you can walk through a list from any node in the list without having a pointer to the list itself. I don't know if we still do, but there used to be a few places where we took advantage of that. Some of the APIs are (very) poorly named (I'm looking at you, insert_before), and I'd welcome patches to fix that up. > Regards, > - Neil > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/13] util/list: Add list_empty and list_length functions
On 05/05/2015 12:21 PM, Neil Roberts wrote: Jason Ekstrand writes: +static inline bool list_empty(struct list_head *list) +{ + return list->next == list; +} It would be good if list.h also included stdbool.h in order to get the declaration of bool. However, will that cause problems on MSVC? Is the Gallium code compiled on MSVC in general? No, and yes. +static inline unsigned list_length(struct list_head *list) +{ + unsigned length = 0; + for (struct list_head *node = list->next; node != list; node = node->next) + length++; + return length; +} Any reason not to use one of the list iterator macros here? Is it safe to use a C99-ism outside of a macro in this header? Maybe MSVC supports this particular C99-ism anyway. I don't remember off-hand. -Brian For what it's worth, I'm strongly in favour of using these kernel-style lists instead of exec_list. The kernel ones seem much less confusing. Regards, - Neil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=AatEjMM7ntWsECW9oVPLk_C3Z0CK7QBIqbXVP_ETF2w&s=TOtSOLlRYSt9tEJikNpITMxBHKDnURqnl6d7RRDfl4w&e= ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/13] util/list: Add list_empty and list_length functions
On Tue, May 5, 2015 at 11:21 AM, Neil Roberts wrote: > Jason Ekstrand writes: > >> +static inline bool list_empty(struct list_head *list) >> +{ >> + return list->next == list; >> +} > > It would be good if list.h also included stdbool.h in order to get the > declaration of bool. However, will that cause problems on MSVC? Is the > Gallium code compiled on MSVC in general? Rob Clark pointed that out. I've fixed it locally. >> +static inline unsigned list_length(struct list_head *list) >> +{ >> + unsigned length = 0; >> + for (struct list_head *node = list->next; node != list; node = >> node->next) >> + length++; >> + return length; >> +} > > Any reason not to use one of the list iterator macros here? Is it safe > to use a C99-ism outside of a macro in this header? Maybe MSVC > supports this particular C99-ism anyway. Because the list iterators work on a subtype and making them work on the node itself is kind of painful. Why C99? No good reason, I could can that easily enough. --Jason > For what it's worth, I'm strongly in favour of using these kernel-style > lists instead of exec_list. The kernel ones seem much less confusing. > > Regards, > - Neil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/13] util/list: Add list_empty and list_length functions
Jason Ekstrand writes: > +static inline bool list_empty(struct list_head *list) > +{ > + return list->next == list; > +} It would be good if list.h also included stdbool.h in order to get the declaration of bool. However, will that cause problems on MSVC? Is the Gallium code compiled on MSVC in general? > +static inline unsigned list_length(struct list_head *list) > +{ > + unsigned length = 0; > + for (struct list_head *node = list->next; node != list; node = node->next) > + length++; > + return length; > +} Any reason not to use one of the list iterator macros here? Is it safe to use a C99-ism outside of a macro in this header? Maybe MSVC supports this particular C99-ism anyway. For what it's worth, I'm strongly in favour of using these kernel-style lists instead of exec_list. The kernel ones seem much less confusing. Regards, - Neil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 09/13] util/list: Add list_empty and list_length functions
--- src/util/list.h | 13 + 1 file changed, 13 insertions(+) diff --git a/src/util/list.h b/src/util/list.h index 6144b0c..246f826 100644 --- a/src/util/list.h +++ b/src/util/list.h @@ -92,6 +92,19 @@ static inline void list_delinit(struct list_head *item) item->prev = item; } +static inline bool list_empty(struct list_head *list) +{ + return list->next == list; +} + +static inline unsigned list_length(struct list_head *list) +{ + unsigned length = 0; + for (struct list_head *node = list->next; node != list; node = node->next) + length++; + return length; +} + #define LIST_INITHEAD(__item) list_inithead(__item) #define LIST_ADD(__item, __list) list_add(__item, __list) #define LIST_ADDTAIL(__item, __list) list_addtail(__item, __list) -- 2.3.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev