mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-09-22 22:31:28 +02:00
Code Issues Projects Releases Wiki Activity

CLEANUP: list: fix again some style issues in the recent comments

Browse Source 
While reading the recent changes around mt_list_for_each_entry_safe() I
noticed a spurious "q" at the beginning of a line introduced by commit
455843721 ("CLEANUP: list: Fix mt_list_for_each_entry_safe indentation")
and that visually confusing multi-line comments missing the trailing '\'
character were introduced by previous commit 60cffbaca ("MINOR: list:
documenting mt_list_for_each_entry_safe() macro"), which at first glance
made the macro look broken. In addition, multi-line comments must end
with a "*/" on its own line to instantly spot where it ends without
having to read the whole line, like this:

    /* we know from the above that foo is always valid
     * here so it's safe to end the string:
     */
    *(unsigned char *)foo = 0;

Not like this:

    /* we know from the above that foo is always valid
     * here so it's safe to end the string: */
    *(unsigned char *)foo = 0;

Finally, macro's main comment mentionned the wrong macro name and types,
and was randomly indented.
This commit is contained in:
Willy Tarreau 2022-09-27 07:42:28 +02:00
parent 70c5ad4425
commit 56ac2cbf58
1 changed files with 47 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

86
include/haproxy/list.h
View File

@ -752,76 +752,84 @@
(_el) = NULL; \ (_el) = NULL; \
} while (0) } while (0)
/* Simpler FOREACH_ITEM_SAFE macro inspired from Linux sources. /* Iterates <item> through a list of items of type "typeof(*item)" which are
* Iterates <item> through a list of items of type "typeof(*item)" which are * linked via a "struct mt_list" member named <member>. A pointer to the head
* linked via a "struct list" member named <member>. A pointer to the head of * of the list is passed in <list_head>.
* the list is passed in <list_head>. *
* tmpelt is a temporary struct mt_list *, and tmpelt2 is a temporary * <tmpelt> is a temporary struct mt_list *, and <tmpelt2> is a temporary
* struct mt_list, used internally, both are needed for MT_LIST_DELETE_SAFE. * struct mt_list, used internally, both are needed for MT_LIST_DELETE_SAFE.
* *
* This macro is implemented using a nested loop: * This macro is implemented using a nested loop. The inner loop will run for
* The inner loop will run for each element in the list, and the upper loop will run * each element in the list, and the upper loop will run only once to do some
* only once to do some cleanup when the end of the list is reached * cleanup when the end of the list is reached or user breaks from inner loop.
* or user breaks from inner loop: * It's safe to break from this macro as the cleanup will be performed anyway,
* you can safely break from this macro as the cleanup will be performed anyway, * but it is strictly forbidden to goto from the loop because skipping the
* but it is strictly forbidden to goto from the loop because skipping the cleanup will * cleanup will lead to undefined behavior.
* lead to undefined behavior.
q *
* If you want to remove the current element, please use MT_LIST_DELETE_SAFE.
* *
* Example: list_for_each_entry_safe(cur_acl, list_head, list_member, elt1, elt2) * In order to remove the current element, please use MT_LIST_DELETE_SAFE.
* { ... }; *
* Example:
* mt_list_for_each_entry_safe(item, list_head, list_member, elt1, elt2) {
* ...
* }
*/ */
#define mt_list_for_each_entry_safe(item, list_head, member, tmpelt, tmpelt2) \ #define mt_list_for_each_entry_safe(item, list_head, member, tmpelt, tmpelt2) \
for ((tmpelt) = NULL; (tmpelt) != MT_LIST_BUSY; ({ \ for ((tmpelt) = NULL; (tmpelt) != MT_LIST_BUSY; ({ \
/* post loop cleanup: /* post loop cleanup: \
* gets executed only once to perform cleanup * gets executed only once to perform cleanup \
* after child loop has finished */ \ * after child loop has finished \
*/ \
if (tmpelt) { \ if (tmpelt) { \
/* last elem still exists, unlocking it */ \ /* last elem still exists, unlocking it */ \
if (tmpelt2.prev) \ if (tmpelt2.prev) \
MT_LIST_UNLOCK_ELT(tmpelt, tmpelt2); \ MT_LIST_UNLOCK_ELT(tmpelt, tmpelt2); \
else { \ else { \
/* special case: child loop did not run /* special case: child loop did not run \
* so tmpelt2.prev == NULL * so tmpelt2.prev == NULL \
* (empty list) */ \ * (empty list) \
*/ \
_MT_LIST_UNLOCK_NEXT(tmpelt, tmpelt2.next); \ _MT_LIST_UNLOCK_NEXT(tmpelt, tmpelt2.next); \
} \ } \
} else { \ } else { \
/* last elem was deleted by user, relink required: \ /* last elem was deleted by user, relink required: \
* prev->next = next * prev->next = next \
* next->prev = prev */ \ * next->prev = prev \
*/ \
_MT_LIST_RELINK_DELETED(tmpelt2); \ _MT_LIST_RELINK_DELETED(tmpelt2); \
} \ } \
/* break parent loop /* break parent loop \
* (this loop runs exactly one time) */ \ * (this loop runs exactly one time) \
*/ \
(tmpelt) = MT_LIST_BUSY; \ (tmpelt) = MT_LIST_BUSY; \
})) \ })) \
for ((tmpelt) = (list_head), (tmpelt2).prev = NULL, (tmpelt2).next = _MT_LIST_LOCK_NEXT(tmpelt); ({ \ for ((tmpelt) = (list_head), (tmpelt2).prev = NULL, (tmpelt2).next = _MT_LIST_LOCK_NEXT(tmpelt); ({ \
/* this gets executed before each user body loop */ \ /* this gets executed before each user body loop */ \
(item) = MT_LIST_ELEM((tmpelt2.next), typeof(item), member); \ (item) = MT_LIST_ELEM((tmpelt2.next), typeof(item), member); \
if (&item->member != (list_head)) { \ if (&item->member != (list_head)) { \
/* did not reach end of list /* did not reach end of list \
* (back to list_head == end of list reached) */ \ * (back to list_head == end of list reached) \
*/ \
if (tmpelt2.prev != &item->member) \ if (tmpelt2.prev != &item->member) \
tmpelt2.next = _MT_LIST_LOCK_NEXT(&item->member); \ tmpelt2.next = _MT_LIST_LOCK_NEXT(&item->member); \
else { \ else { \
/* FIXME: is this even supposed to happen?? /* FIXME: is this even supposed to happen?? \
* I'm not understanding how * I'm not understanding how \
* tmpelt2.prev could be equal to &item->member. * tmpelt2.prev could be equal to &item->member. \
* running 'test_list' multiple times with 8 * running 'test_list' multiple times with 8 \
* concurrent threads: this never gets reached */ \ * concurrent threads: this never gets reached \
*/ \
tmpelt2.next = tmpelt; \ tmpelt2.next = tmpelt; \
} \ } \
if (tmpelt != NULL) { \ if (tmpelt != NULL) { \
/* if tmpelt was not deleted by user */ \ /* if tmpelt was not deleted by user */ \
if (tmpelt2.prev) { \ if (tmpelt2.prev) { \
/* not executed on first run /* not executed on first run \
* (tmpelt2.prev == NULL on first run) */ \ * (tmpelt2.prev == NULL on first run) \
*/ \
_MT_LIST_UNLOCK_PREV(tmpelt, tmpelt2.prev); \ _MT_LIST_UNLOCK_PREV(tmpelt, tmpelt2.prev); \
/* unlock_prev will implicitely relink: /* unlock_prev will implicitely relink: \
* elt->prev = prev * elt->prev = prev \
* prev->next = elt * prev->next = elt \
*/ \ */ \
} \ } \
tmpelt2.prev = tmpelt; \ tmpelt2.prev = tmpelt; \