On 09/06/2012 06:50 PM, Mathieu Desnoyers wrote: > * Sasha Levin (levinsasha928@xxxxxxxxx) wrote: >> On 09/06/2012 06:00 PM, Steven Rostedt wrote: >>>>> I think that that code doesn't make sense. The users of hlist_for_each_* aren't >>>>> supposed to be changing the loop cursor. >>> I totally agree. Modifying the 'node' pointer is just asking for issues. >>> Yes that is error prone, but not due to the double loop. It's due to the >>> modifying of the node pointer that is used internally by the loop >>> counter. Don't do that :-) >> >> While we're on this subject, I haven't actually seen hlist_for_each_entry() code >> that even *touches* 'pos'. >> >> Will people yell at me loudly if I change the prototype of those macros to be: >> >> hlist_for_each_entry(tpos, head, member) >> >> (Dropping the 'pos' parameter), and updating anything that calls those macros to >> drop it as well? > > I think the intent there is to keep hlist macros and list macros > slightly in sync. Given those are vastly used, I'm not sure you want to > touch them. But hey, that's just my 2 cents. Actually, the corresponding list macro looks like this: list_for_each_entry(pos, head, member) With 'pos' being the equivalent of 'tpos' in the hlist macros (the type *). Changing hlist macro will make them both look as follows: hlist_for_each_entry(pos, head, member) list_for_each_entry(pos, head, member) So following this suggesting will actually bring them back to sync... The only issue I can see is that as you've said, they're used almost everywhere, so doing something to change that will require some coordination. Thanks, Sasha -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
