Neil Brown wrote:
> On Wednesday July 16, SteveD@xxxxxxxxxx wrote:
>>> Arguably it would be cleaner architecturally if the DNS lookup were in
>>> notify_host(). Since doing it in notify() means you will be looking
>>> up these addresses on retries, maybe the host_lookup() call should be
>>> integrated into the "If we retransmitted 4 times" logic. That would
>>> be an opportunity to fix the addrinfo leak in there.
>
> What addrinfo leak is that?
See questions below...
>
>>> [Note that freeaddrinfo(3) simply walks the list of addrinfo
>>> structures and frees them. By setting ai_next to NULL in some of the
>>> addrinfo structures, sm-notify is orphaning them -- freeaddrinfo(3)
>>> will never find them].
>>>
>
> I assume we are talking about the code in notify_host in the
> if (host->retries >= 4) {
> branch.
>
> What the code is doing is taking the first addrinfo from the host->ai
> list, and moving it to the end - effectively rotating the list.
> This shouldn't change what freeaddrinfo will do.
>
> One of us is missing something.
>>
>> I believe the following patch address those minor comments... true?
>
> Except for:
>
>> @@ -349,7 +364,6 @@ notify_host(int sock, struct nsm_host *host)
>> while ( *next )
>> next = & (*next)->ai_next;
>> *next = hold;
>> - hold->ai_next = NULL;
>> memcpy(&host->addr, hold->ai_addr, hold->ai_addrlen);
>> addr_set_port(&host->addr, 0);
>> host->retries = 0;
>
> which I think is wrong, and wondering why:
After the while loop doesn't hold point to the head of the list?
If so, setting hold->ai_next = NULL; orphans the rest of the list, right?
Or am I missing something...
steved.
--
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
