Messages in this thread | | | Date | Thu, 06 Sep 2012 19:01:18 +0200 | From | Sasha Levin <> | Subject | Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable |
| |
On 09/06/2012 06:50 PM, Mathieu Desnoyers wrote: > * Sasha Levin (levinsasha928@gmail.com) 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
| |