[PATCH] tevent_common_context_destructor aims to remove all node from list?

[boyang]

Jeremy Allison wrote:
> On Sat, Jan 10, 2009 at 01:54:41PM +0800, boyang wrote:
>   
>> Hi, Metze:
>>      Does tevent_common_context_destructor aim to remove all nodes
>> instead of just the first one? I think it is your intention to remove
>> all instead of the first one. The current code just remove the first
>> one, patch tries to fix it. Please review it.
>>      Sorry that I have not compile it at present, because I don't have a
>> build environment at my hand now. :-)
>>     
>
> Arg. I know that patch is correct and metze just committed it,
> but I *hate* that :
>
> ptr = list_head
> while(ptr) {
> 	do stuff...
> 	ptr = ptr->next;
> }
>
> idiom. This code was the cause of the bug in the
> krb5 creds reinitialization code you back ported
> that I missed when reviewing the back port (the
>
> ptr = ptr->next;
>
> part was missing). That's an easy bug to introduce
> by mistake.
>
> It's a *for* loop dammit :-). Please re-code as
>
> for (ptr = list_head; ptr; ptr = ptr->next) {
> 	do stuff..
> }
>
> as that makes much more sense (and indeed was
> the way it was originally written). The only time
> you need to do the while() idiom is if you're
> deleting elements of the list and can't use
> ptr = ptr->next as the last part of the for
> as the ptr element was freed.
>   
:-). We do are deleting elements of the list here. :-) If we use for
statement here, it is:
for (ptr = list_head; ptr; ) {
    doing stuff;
    DLIST_REMOVE(list_head, ptr);
    ptr = list_head;
}
> Just a small rant, don't take it personally :-) :-).
>   
:-) Problems are problems. it is not related to any personal stuffs.
Suggestions and critics are welcome. :-)
> Cheers,
>
> Jeremy.
>
>   

-------------- next part --------------
A non-text attachment was scrubbed...
Name: boyang.vcf
Type: text/x-vcard
Size: 187 bytes
Desc: not available
Url : http://lists.samba.org/archive/samba-technical/attachments/20090112/cb2822f9/boyang.vcf