[lttng-dev] Problem in userspace-rcu cds_hlist_del_rcu.c caused by #define cds_hlist_for_each_entry_safe_2

[Thibault, Daniel]

   When compiling userspace-rcu (commit 81dd913), I get a pair of warnings for /userspace-rcu/doc/examples/hlist/cds_hlist_del_rcu.c at the cds_hlist_for_each_entry_safe_2 call:

	cds_hlist_for_each_entry_safe_2(node, n, &mylist, node) {
		if (node->value > 0) {
			cds_hlist_del_rcu(&node->node);
			/*
			 * We can only reclaim memory after a grace
			 * period has passed after cds_hlist_del_rcu().
			 */
			call_rcu(&node->rcu_head, free_node_rcu);
		}
	}

   The warnings are:
"attention: assignment makes pointer from integer without a cast"
"attention: left operand of comma operator has no effect"

   The #define comes from userspace-rcu/urcu/hlist.h (here re-arranged a bit):

#define cds_hlist_for_each_entry_safe_2(entry, e, head, member) \
	for (entry = ((head)->next == NULL ? NULL : cds_hlist_entry((head)->next, __typeof__(*entry), member)); \
	        (entry != NULL) && \
	        (e = (entry->member.next == NULL ? NULL : cds_hlist_entry(entry->member.next, __typeof__(*entry), member), 1)); \
	        entry = e)

   The cds_hlist_del_rcu.c occurrence is the only use of cds_hlist_for_each_entry_safe_2 and the problem seems to me to lie in this bit:

(e = (entry->member.next == NULL ? NULL : cds_hlist_entry(entry->member.next, __typeof__(*entry), member), 1))

   Because the comma operator has the lowest precedence of all operators, the statement is equivalent to:

(e = ((entry->member.next == NULL ? NULL : cds_hlist_entry(entry->member.next, __typeof__(*entry), member)), 1)) 

   Which is in turn the same as:

(e = 1)

   Now, if the macro is using the comma to make the second part of the && test succeed regardless of the value fetched for e, then it should have been written:

#define cds_hlist_for_each_entry_safe_2(entry, e, head, member) \
	for (entry = ((head)->next == NULL ? NULL : cds_hlist_entry((head)->next, __typeof__(*entry), member)); \
	        (entry != NULL) && \
	        ((e = (entry->member.next == NULL ? NULL : cds_hlist_entry(entry->member.next, __typeof__(*entry), member))), 1); \
	        entry = e)

   Which does compile without the warnings.

Daniel U. Thibault
Protection des systèmes et contremesures (PSC) | Systems Protection & Countermeasures (SPC)
Cyber sécurité pour les missions essentielles (CME) | Mission Critical Cyber Security (MCCS)
R & D pour la défense Canada - Valcartier (RDDC Valcartier) | Defence R&D Canada - Valcartier (DRDC Valcartier)
2459 route de la Bravoure
Québec QC  G3J 1X5
CANADA
Vox : (418) 844-4000 x4245
Fax : (418) 844-4538
NAC : 918V QSDJ <http://www.travelgis.com/map.asp?addr=918V%20QSDJ>
Gouvernement du Canada | Government of Canada
<http://www.valcartier.drdc-rddc.gc.ca/>