Re: [PATCH wayland v6] util: Document wl_list methods
On Thu, 22 Sep 2016 22:11:39 -0700 Bryce Harringtonwrote: > On Thu, Sep 22, 2016 at 09:59:37PM -0500, Yong Bakos wrote: > > From: Yong Bakos > > > > Add doxygen comment blocks to all wl_list methods. > > > > Signed-off-by: Yong Bakos > > Reviewed-by: Bryce Harrington > > One extremely minor wording suggestion below only if you do another rev. > But this all looks extremely good; will be nice to get the wl_list > documentation shaped up, thanks. > > Bryce > > > --- > > v6: Change description to doubly-linked list (pq) > > v5: Change description to linked-list [err] > > Clarify uses of `wl_list_init` (pq) > > v4: Fix variable name in sample code. (pq) > > Remove implemenetation details of pointer state. (pq) > > Remove note about __typeof__ entirely. > > - it's not helpful as a note or a code comment either > > Change sample code indentation to just spaces. (pq) > > Use _list suffix for list in sample code. (pq) > > Use 'iterate' instead of enumerate, for consistency. (pq) > > Note that only removing 'pos' is safe for *_safe methods. (pq, giucam) > > v3: Standardize on term 'element', to match param names, tests, and other > > text. > > Use 'relates' for macros (instead of 'memberof'). > > v2: Refine the writing for clarity. > > Add dox for wl_list macros, omitted in v1. > > Add notices for unsafe operations and invalid states (giucam, pq) > > src/wayland-util.h | 224 > > +++-- > > 1 file changed, 184 insertions(+), 40 deletions(-) > > > > +/** > > + * Inserts all of the elements of one list into another, after the element > > + * represented by \p list. > > + * > > + * \note This leaves \p other itself in an invalid state. > > Might read better if you take the word 'itself' out? > Hi, I made the change Bryce suggested and pushed the result: 454df51..f7e1dcc master -> master Thanks, pq pgp8dMvnr1qCm.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v6] util: Document wl_list methods
On Thu, Sep 22, 2016 at 09:59:37PM -0500, Yong Bakos wrote: > From: Yong Bakos> > Add doxygen comment blocks to all wl_list methods. > > Signed-off-by: Yong Bakos Reviewed-by: Bryce Harrington One extremely minor wording suggestion below only if you do another rev. But this all looks extremely good; will be nice to get the wl_list documentation shaped up, thanks. Bryce > --- > v6: Change description to doubly-linked list (pq) > v5: Change description to linked-list [err] > Clarify uses of `wl_list_init` (pq) > v4: Fix variable name in sample code. (pq) > Remove implemenetation details of pointer state. (pq) > Remove note about __typeof__ entirely. > - it's not helpful as a note or a code comment either > Change sample code indentation to just spaces. (pq) > Use _list suffix for list in sample code. (pq) > Use 'iterate' instead of enumerate, for consistency. (pq) > Note that only removing 'pos' is safe for *_safe methods. (pq, giucam) > v3: Standardize on term 'element', to match param names, tests, and other > text. > Use 'relates' for macros (instead of 'memberof'). > v2: Refine the writing for clarity. > Add dox for wl_list macros, omitted in v1. > Add notices for unsafe operations and invalid states (giucam, pq) > src/wayland-util.h | 224 > +++-- > 1 file changed, 184 insertions(+), 40 deletions(-) > > diff --git a/src/wayland-util.h b/src/wayland-util.h > index cacc122..71c26a1 100644 > --- a/src/wayland-util.h > +++ b/src/wayland-util.h > @@ -78,73 +78,150 @@ struct wl_interface { > > /** \class wl_list > * > - * \brief doubly-linked list > + * \brief Doubly-linked list > * > - * The list head is of "struct wl_list" type, and must be initialized > - * using wl_list_init(). All entries in the list must be of the same > - * type. The item type must have a "struct wl_list" member. This > - * member will be initialized by wl_list_insert(). There is no need to > - * call wl_list_init() on the individual item. To query if the list is > - * empty in O(1), use wl_list_empty(). > + * On its own, an instance of `struct wl_list` represents the sentinel head > of > + * a doubly-linked list, and must be initialized using wl_list_init(). > + * When empty, the list head's `next` and `prev` members point to the list > head > + * itself, otherwise `next` references the first element in the list, and > `prev` > + * refers to the last element in the list. > * > - * Let's call the list reference "struct wl_list foo_list", the item type as > - * "item_t", and the item member as "struct wl_list link". > + * Use the `struct wl_list` type to represent both the list head and the > links > + * between elements within the list. Use wl_list_empty() to determine if the > + * list is empty in O(1). > + * > + * All elements in the list must be of the same type. The element type must > have > + * a `struct wl_list` member, often named `link` by convention. Prior to > + * insertion, there is no need to initialize an element's `link` - invoking > + * wl_list_init() on an individual list element's `struct wl_list` member is > + * unnecessary if the very next operation is wl_list_insert(). However, a > + * common idiom is to initialize an element's `link` prior to removal - > ensure > + * safety by invoking wl_list_init() before wl_list_remove(). > + * > + * Consider a list reference `struct wl_list foo_list`, an element type as > + * `struct element`, and an element's link member as `struct wl_list link`. > + * > + * The following code initializes a list and adds three elements to it. > * > - * The following code will initialize a list: > * \code > * struct wl_list foo_list; > * > - * struct item_t { > - * int foo; > - * struct wl_list link; > + * struct element { > + * int foo; > + * struct wl_list link; > * }; > - * struct item_t item1, item2, item3; > + * struct element e1, e2, e3; > * > * wl_list_init(_list); > - * wl_list_insert(_list, ); // Pushes item1 at the head > - * wl_list_insert(_list, ); // Pushes item2 at the head > - * wl_list_insert(, ); // Pushes item3 after item2 > + * wl_list_insert(_list, ); // e1 is the first element > + * wl_list_insert(_list, ); // e2 is now the first element > + * wl_list_insert(, ); // insert e3 after e2 > * \endcode > > - * The list now looks like [item2, item3, item1] > + * The list now looks like [e2, e3, e1]. > + * > + * The `wl_list` API provides some iterator macros. For example, to iterate > + * a list in ascending order: > * > - * Iterate the list in ascending order: > * \code > - * item_t *item; > - * wl_list_for_each(item, foo_list, link) { > - * Do_something_with_item(item); > + * struct element *e; > + * wl_list_for_each(e, foo_list, link) { > + * do_something_with_element(e); > * } > * \endcode > + * > + * See the
[PATCH wayland v6] util: Document wl_list methods
From: Yong BakosAdd doxygen comment blocks to all wl_list methods. Signed-off-by: Yong Bakos --- v6: Change description to doubly-linked list (pq) v5: Change description to linked-list [err] Clarify uses of `wl_list_init` (pq) v4: Fix variable name in sample code. (pq) Remove implemenetation details of pointer state. (pq) Remove note about __typeof__ entirely. - it's not helpful as a note or a code comment either Change sample code indentation to just spaces. (pq) Use _list suffix for list in sample code. (pq) Use 'iterate' instead of enumerate, for consistency. (pq) Note that only removing 'pos' is safe for *_safe methods. (pq, giucam) v3: Standardize on term 'element', to match param names, tests, and other text. Use 'relates' for macros (instead of 'memberof'). v2: Refine the writing for clarity. Add dox for wl_list macros, omitted in v1. Add notices for unsafe operations and invalid states (giucam, pq) src/wayland-util.h | 224 +++-- 1 file changed, 184 insertions(+), 40 deletions(-) diff --git a/src/wayland-util.h b/src/wayland-util.h index cacc122..71c26a1 100644 --- a/src/wayland-util.h +++ b/src/wayland-util.h @@ -78,73 +78,150 @@ struct wl_interface { /** \class wl_list * - * \brief doubly-linked list + * \brief Doubly-linked list * - * The list head is of "struct wl_list" type, and must be initialized - * using wl_list_init(). All entries in the list must be of the same - * type. The item type must have a "struct wl_list" member. This - * member will be initialized by wl_list_insert(). There is no need to - * call wl_list_init() on the individual item. To query if the list is - * empty in O(1), use wl_list_empty(). + * On its own, an instance of `struct wl_list` represents the sentinel head of + * a doubly-linked list, and must be initialized using wl_list_init(). + * When empty, the list head's `next` and `prev` members point to the list head + * itself, otherwise `next` references the first element in the list, and `prev` + * refers to the last element in the list. * - * Let's call the list reference "struct wl_list foo_list", the item type as - * "item_t", and the item member as "struct wl_list link". + * Use the `struct wl_list` type to represent both the list head and the links + * between elements within the list. Use wl_list_empty() to determine if the + * list is empty in O(1). + * + * All elements in the list must be of the same type. The element type must have + * a `struct wl_list` member, often named `link` by convention. Prior to + * insertion, there is no need to initialize an element's `link` - invoking + * wl_list_init() on an individual list element's `struct wl_list` member is + * unnecessary if the very next operation is wl_list_insert(). However, a + * common idiom is to initialize an element's `link` prior to removal - ensure + * safety by invoking wl_list_init() before wl_list_remove(). + * + * Consider a list reference `struct wl_list foo_list`, an element type as + * `struct element`, and an element's link member as `struct wl_list link`. + * + * The following code initializes a list and adds three elements to it. * - * The following code will initialize a list: * \code * struct wl_list foo_list; * - * struct item_t { - * int foo; - * struct wl_list link; + * struct element { + * int foo; + * struct wl_list link; * }; - * struct item_t item1, item2, item3; + * struct element e1, e2, e3; * * wl_list_init(_list); - * wl_list_insert(_list, ); // Pushes item1 at the head - * wl_list_insert(_list, ); // Pushes item2 at the head - * wl_list_insert(, ); // Pushes item3 after item2 + * wl_list_insert(_list, ); // e1 is the first element + * wl_list_insert(_list, ); // e2 is now the first element + * wl_list_insert(, ); // insert e3 after e2 * \endcode * - * The list now looks like [item2, item3, item1] + * The list now looks like [e2, e3, e1]. + * + * The `wl_list` API provides some iterator macros. For example, to iterate + * a list in ascending order: * - * Iterate the list in ascending order: * \code - * item_t *item; - * wl_list_for_each(item, foo_list, link) { - * Do_something_with_item(item); + * struct element *e; + * wl_list_for_each(e, foo_list, link) { + * do_something_with_element(e); * } * \endcode + * + * See the documentation of each iterator for details. + * \sa http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/linux/list.h */ struct wl_list { struct wl_list *prev; struct wl_list *next; }; +/** + * Initializes the list. + * + * \param list List to initialize + * + * \memberof wl_list + */ void wl_list_init(struct wl_list *list); +/** + * Inserts an element into the list, after the element represented by \p list. + * When \p list is a reference to the list itself (the head), set