[Dnsmasq-discuss] Dnsmasq with Gigantic hosts file

[Jan 'RedBully' Seiffert]

Simon Kelley wrote:
> Jan 'RedBully' Seiffert wrote:
>> Simon Kelley wrote:
[snip]
>>> I've not tested this yet, but I think the following patch should be enough.
>> [snip - patch]
>>
>> But doesn't this sorting clash with the resorting of cache_find_by_name
>> (according to line 487)?
> 
> The cache entries are on two different linked lists, the
> least-recently-used one doubly-linked on ->next and ->prev and the
> hash-list, linked on ->hash_next. You are thinking about the LRU list
> but I'm talking about the hash-list.
> 
*plop* *large light bulb appears over head*
Ahhh, now i see. Ok, i simply didn't grasp that there are too listed
which get maintained. I thought the LRU thing is within the hash bucket...

> This code has evolved slowly over a long time and has all sorts of scary
> stuff like that. I'd do it very differently from scratch but what's
> there 1) works well and fast. 2) had the bugs shaken out long ago. so I
> don't feel inclined to toss it.
> 
No prob. I'm with you on this, because thats also what i saw the last
two years using dnsmasq: "It simply works"(TM)

[snip - insert cost]
>> I will also test it, time to update to the last version...
>>
> 
> Thanks. 2.36 didn't change cache.c, AFAIR, so the patch should apply to
> 2.35 or 2.36.
> 
OK, i tried to test it, but it fails in so far, that i didn't get any
entries into the cache besides those from /etc/hosts and dhcp.
After i sprinkled some printf in the code, i think i found a genuine bug.
The compiler would call it: "the variable is_sign might get used
uninitialized".

forward.c, process_reply at 351 && 358:
is_sign is declared, but not initialized, and then passed "by reference"
to find_pseudoheader
rfc1035.c find_pseudoheader at 434 && 475:
under special circumstances is_sign gets a value, otherwise it is left
'as is'
forward.c, process_reply at 358 && 368:
is_sign is checked == 0, but may contain total garbage from the stack:
dnsmasq: forward.c process_reply
dnsmasq: forward.c process_reply before test, is_sign -1208760160
dnsmasq: forward.c process_reply
dnsmasq: forward.c process_reply before test, is_sign 134615120

This problem may also exist in other places.
....
I made a little patch, but maybe its wiser to set is_sign to zero in
find_pseudoheader, i don't know.

With this patch, i will go back to testing...

[snip - patch approach]
> (but I need to worry on concurrent access, coding on dnsmasq is
>> mind relaxing in this regard ;)
> 
> a select-loop is a wonderful thing ;-)
> 
I prefer epoll(7)-loops ;-)
Still i deliberately choose to split it up in some main threads to help
scalability, because i want to handle $BIGNUM longstanding simultaneous
connections with UDP-Traffic intermixed. But even if wisely chosen where
to split things up, somewhere you need concurrent access...

> Cheers,
> 
> Simon.
> 
> 
> 
Greetings
	Jan

-- 
H.323 has much in common with other ITU-T standards - it features a
complex binary wire protocol, a nightmarish implementation, and a bulk
that can be used to fell medium-to-large predatory animals.
        -- Anthony Baxter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dnsmasq-2.36-uninitialized.diff
Type: text/x-patch
Size: 1556 bytes
Desc: not available
Url : http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/attachments/20070130/9c467f8c/dnsmasq-2.36-uninitialized-0001.bin