[vlc-devel] [PATCH 4/9] input:info: use a name for the node that reflects which list fills it

Steve Lhomme robux4 at ycbcr.xyz
Fri Aug 17 15:16:01 CEST 2018


You mean without the comment ? The renaming or both ?

Personally naming a node 'node' is as useful as naming a struct 'struct' 
or an int 'int'. We can do better than that. And in the case of 
intrusive list it's especially important to know who's going to write in 
there. And it also helps to find the right field when using the item. If 
we use a convention like I used (listname_item) we could even have 
macros that use just the list name, the field name of the nodes will be 
implied.


On 17/08/2018 15:09, Thomas Guillem wrote:
> I don't think that it is useful if there is only one node in the struct.
>
> On Fri, Aug 17, 2018, at 15:04, Steve Lhomme wrote:
>> ---
>>   include/vlc_input_item.h                  |  6 +++---
>>   modules/gui/qt/components/info_panels.cpp |  4 ++--
>>   src/input/info.h                          | 12 ++++++------
>>   3 files changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/vlc_input_item.h b/include/vlc_input_item.h
>> index 334e81f3e1..1ba4b08483 100644
>> --- a/include/vlc_input_item.h
>> +++ b/include/vlc_input_item.h
>> @@ -44,15 +44,15 @@ struct info_t
>>   {
>>       char *psz_name;            /**< Name of this info */
>>       char *psz_value;           /**< Value of the info */
>> -    vlc_list_node node;        /* from list info_category_t.infos */
>> +    vlc_list_node infos_item;    /* from list info_category_t.infos */
>>   };
>>   
>> -#define info_foreach(info, cat) vlc_list_foreach(info, cat, node)
>> +#define info_foreach(info, cat) vlc_list_foreach(info, cat, infos_item)
>>   
>>   struct info_category_t
>>   {
>>       char   *psz_name;      /**< Name of this category */
>> -    struct vlc_list infos; /**< Infos in the category, intrusive in
>> info_t.node */
>> +    struct vlc_list infos; /**< Infos in the category, intrusive in
>> info_t.infos_item */
>>   };
>>   
>>   enum input_item_type_e
>> diff --git a/modules/gui/qt/components/info_panels.cpp b/modules/gui/qt/
>> components/info_panels.cpp
>> index 4f86cb7f80..723f92b5b0 100644
>> --- a/modules/gui/qt/components/info_panels.cpp
>> +++ b/modules/gui/qt/components/info_panels.cpp
>> @@ -508,9 +508,9 @@ void InfoPanel::update( input_item_t *p_item)
>>           current_item->setText( 0, qfu(p_item->pp_categories[i]-
>>> psz_name) );
>>           InfoTree->addTopLevelItem( current_item );
>>   
>> -        for (info_t *info = vlc_list_first_entry_or_null(head, info_t,
>> node);
>> +        for (info_t *info = vlc_list_first_entry_or_null(head, info_t,
>> infos_item);
>>                info != NULL;
>> -             info = vlc_list_next_entry_or_null(head, info, info_t,
>> node))
>> +             info = vlc_list_next_entry_or_null(head, info, info_t,
>> infos_item))
>>           {
>>               child_item = new QTreeWidgetItem ();
>>               child_item->setText( 0, qfu(info->psz_name) + ": "
>> diff --git a/src/input/info.h b/src/input/info.h
>> index e32a003b93..ce394ed699 100644
>> --- a/src/input/info.h
>> +++ b/src/input/info.h
>> @@ -70,10 +70,10 @@ static inline void
>> info_category_ReplaceInfo(info_category_t *cat,
>>   {
>>       info_t *old = info_category_FindInfo(cat, info->psz_name);
>>       if (old) {
>> -        vlc_list_remove(&old->node);
>> +        vlc_list_remove(&old->infos_item);
>>           info_Delete(old);
>>       }
>> -    vlc_list_append(&info->node, &cat->infos);
>> +    vlc_list_append(&info->infos_item, &cat->infos);
>>   }
>>   
>>   static inline info_t *info_category_VaAddInfo(info_category_t *cat,
>> @@ -85,7 +85,7 @@ static inline info_t
>> *info_category_VaAddInfo(info_category_t *cat,
>>           info = info_New(name);
>>           if (!info)
>>               return NULL;
>> -        vlc_list_append(&info->node, &cat->infos);
>> +        vlc_list_append(&info->infos_item, &cat->infos);
>>       } else
>>           free(info->psz_value);
>>       if (vasprintf(&info->psz_value, format, args) == -1)
>> @@ -110,7 +110,7 @@ static inline int
>> info_category_DeleteInfo(info_category_t *cat, const char *nam
>>   {
>>       info_t *info = info_category_FindInfo(cat, name);
>>       if (info != NULL) {
>> -        vlc_list_remove(&info->node);
>> +        vlc_list_remove(&info->infos_item);
>>           info_Delete(info);
>>           return VLC_SUCCESS;
>>       }
>> @@ -121,8 +121,8 @@ static inline void
>> info_category_Delete(info_category_t *cat)
>>   {
>>       info_t *info;
>>   
>> -    while ((info = vlc_list_first_entry_or_null(&cat->infos, info_t,
>> node))) {
>> -        vlc_list_remove(&info->node);
>> +    while ((info = vlc_list_first_entry_or_null(&cat->infos, info_t,
>> infos_item))) {
>> +        vlc_list_remove(&info->infos_item);
>>           info_Delete(info);
>>       }
>>       free(cat->psz_name);
>> -- 
>> 2.17.0
>>
>> _______________________________________________
>> 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



More information about the vlc-devel mailing list