[vlc-devel] [PATCH 1/9] vlc_list: add a typedef vlc_list_node to clarify which is the list and which is the node

Steve Lhomme robux4 at ycbcr.xyz
Mon Aug 20 12:44:11 CEST 2018


On 20/08/2018 12:03, Rémi Denis-Courmont wrote:
> No, it is not possible to distinguish the type of a node and a head in 
> a circular list, in a statically typed (only) language. You need 
> dynamic typing for that to work.

I think you're talking about the type. I didn't look too much into the 
implementation (because one shouldn't). But for the elements where the 
list is stored and where the elements are it does make a difference at 
least for the reader.

The vlc_list also doesn't say that it's circular. So it seems the intent 
was not to tell about implementation details. So I don't see why the 
user should have to know they're interchangeable (maybe one day they won't).

> Adding two different type names for the same thing is confusing. This 
> patch series makes things much more confusing, or rather confused 
> even, to me. And I happen to be the author...

Agree to disagree.

>
> Le 20 août 2018 10:21:38 GMT+03:00, Steve Lhomme <robux4 at ycbcr.xyz> a 
> écrit :
>
>     On 17/08/2018 19:06, Rémi Denis-Courmont wrote:
>
>         You cannot have a different typedef here. This is by design.
>         And we already use different member names for heads and nodes. 
>
>
>     It's possible, it's even desirable. "vlc_list node" is the part that
>     doesn't make sense here. A node is not a list, the fact it shares the
>     same internals is because of the current design. It may change in the
>     future, but what is 100% is that a node must not be used as a vlc_list
>     despite its type. And so it should not imply it can be from its type
>     (maybe the mnemonic is wrong, who knows).
>
>         Le 17 août 2018 16:04:33 GMT+03:00, Steve Lhomme
>         <robux4 at ycbcr.xyz> a écrit : --- include/vlc_list.h | 23
>         +++++++++++++---------- 1 file changed, 13 insertions(+), 10
>         deletions(-) diff --git a/include/vlc_list.h
>         b/include/vlc_list.h index 45263cc789..002119db6b 100644 ---
>         a/include/vlc_list.h +++ b/include/vlc_list.h @@ -45,6 +45,9
>         @@ struct vlc_list struct vlc_list *next; }; +/* type to use
>         in list items to differentiate with the list itself */
>         +typedef struct vlc_list vlc_list_node; + /** * Static
>         initializer for a list head. */ @@ -66,7 +69,7 @@ static
>         inline void vlc_list_init(struct vlc_list *restrict head) *
>         \param prev Node pointer of the previous element. * \param
>         next Node pointer of the next element. */ -static inline void
>         vlc_list_add_between(struct vlc_list *restrict node, +static
>         inline void vlc_list_add_between(vlc_list_node *restrict node,
>         struct vlc_list *prev, struct vlc_list *next) { @@ -82,7 +85,7
>         @@ static inline void vlc_list_add_between(struct vlc_list
>         *restrict node, * \param node Node pointer of the element to
>         insert [OUT]. * \param prev Node pointer of the previous
>         element. */ -static inline void vlc_list_add_after(struct
>         vlc_list *restrict node, +static inline void
>         vlc_list_add_after(vlc_list_node *restrict node, struct
>         vlc_list *prev) { vlc_list_add_between(node, prev,
>         prev->next); @@ -94,7 +97,7 @@ static inline void
>         vlc_list_add_after(struct vlc_list *restrict node, * \param
>         node Node pointer of the element to insert [OUT]. * \param
>         prev Node pointer of the next element. */ -static inline void
>         vlc_list_add_before(struct vlc_list *restrict node, +static
>         inline void vlc_list_add_before(vlc_list_node *restrict node,
>         struct vlc_list *next) { vlc_list_add_between(node,
>         next->prev, next); @@ -106,7 +109,7 @@ static inline void
>         vlc_list_add_before(struct vlc_list *restrict node, * \param
>         node Node pointer of the element to append to the list [OUT].
>         * \param head Head pointer of the list to append the element
>         to. */ -static inline void vlc_list_append(struct vlc_list
>         *restrict node, +static inline void
>         vlc_list_append(vlc_list_node *restrict node, struct vlc_list
>         *head) { vlc_list_add_before(node, head); @@ -118,7 +121,7 @@
>         static inline void vlc_list_append(struct vlc_list *restrict
>         node, * \param node Node pointer of the element to prepend to
>         the list [OUT]. * \param head Head pointer of the list to
>         prepend the element to. */ -static inline void
>         vlc_list_prepend(struct vlc_list *restrict node, +static
>         inline void vlc_list_prepend(vlc_list_node *restrict node,
>         struct vlc_list *head) { vlc_list_add_after(node, head); @@
>         -131,7 +134,7 @@ static inline void vlc_list_prepend(struct
>         vlc_list *restrict node, * \warning The element must be inside
>         a list. * Otherwise the behaviour is undefined. */ -static
>         inline void vlc_list_remove(struct vlc_list *restrict node)
>         +static inline void vlc_list_remove(vlc_list_node *restrict
>         node) { struct vlc_list *prev = node->prev; struct vlc_list
>         *next = node->next; @@ -177,7 +180,7 @@ static inline bool
>         vlc_list_is_empty(const struct vlc_list *head) * \retval false
>         The element is not first (or is in another list). * \retval
>         true The element is first. */ -static inline bool
>         vlc_list_is_first(const struct vlc_list *node, +static inline
>         bool vlc_list_is_first(const vlc_list_node *node, const struct
>         vlc_list *head) { return node->prev == head; @@ -192,7 +195,7
>         @@ static inline bool vlc_list_is_first(const struct vlc_list
>         *node, * \retval false The element is not last (or is in
>         another list). * \retval true The element is last. */ -static
>         inline bool vlc_list_is_last(const struct vlc_list *node,
>         +static inline bool vlc_list_is_last(const vlc_list_node
>         *node, const struct vlc_list *head) { return node->next ==
>         head; @@ -289,7 +292,7 @@ static inline void
>         *vlc_list_last_or_null(const struct vlc_list *head, } static
>         inline void *vlc_list_prev_or_null(const struct vlc_list
>         *head, - struct vlc_list *node, + vlc_list_node *node, size_t
>         offset) { if (vlc_list_is_first(node, head)) @@ -298,7 +301,7
>         @@ static inline void *vlc_list_prev_or_null(const struct
>         vlc_list *head, } static inline void
>         *vlc_list_next_or_null(const struct vlc_list *head, - struct
>         vlc_list *node, + vlc_list_node *node, size_t offset) { if
>         (vlc_list_is_last(node, head)) -- Envoyé de mon appareil
>         Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
>         ------------------------------------------------------------------------
>         vlc-devel mailing list To unsubscribe or modify your
>         subscription options:
>         https://mailman.videolan.org/listinfo/vlc-devel 
>
>
>     ------------------------------------------------------------------------
>
>     vlc-devel mailing list
>     To unsubscribe or modify your subscription options:
>     https://mailman.videolan.org/listinfo/vlc-devel
>
>
> -- 
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez 
> excuser ma brièveté.
>
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel



More information about the vlc-devel mailing list