From 7fc300d5dc8e355b410f9810e4550a7b36c170f5 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Sat, 14 Jul 2007 13:11:36 +0000 Subject: added Vlad's entire description of his valgrind fix --- ares/CHANGES | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) (limited to 'ares') diff --git a/ares/CHANGES b/ares/CHANGES index fcebe4ea4..86d46ed44 100644 --- a/ares/CHANGES +++ b/ares/CHANGES @@ -2,7 +2,36 @@ * July 14 2007 (Daniel Stenberg) -- Vlad Dinulescu fixed two outstanding valgrind reports. +- Vlad Dinulescu fixed two outstanding valgrind reports: + + + 1. In ares_query.c , in find_query_by_id we compare q->qid (which is a short + int variable) with qid, which is declared as an int variable. Moreover, + DNS_HEADER_SET_QID is used to set the value of qid, but DNS_HEADER_SET_QID + sets only the first two bytes of qid. I think that qid should be declared as + "unsigned short" in this function. + + 2. The same problem occurs in ares_process.c, process_answer() . query->qid + (an unsigned short integer variable) is compared with id, which is an + integer variable. Moreover, id is initialized from DNS_HEADER_QID which sets + only the first two bytes of id. I think that the id variable should be + declared as "unsigned short" in this function. + + Even after declaring these variables as "unsigned short", the valgrind + errors are still there. Which brings us to the third problem. + + 3. The third problem is that Valgrind assumes that query->qid is not + initialised correctly. And it does that because query->qid is set from + DNS_HEADER_QID(qbuf); Valgrind says that qbuf has unitialised bytes. And + qbuf has uninitialised bytes because of channel->next_id . And next_id is + set by ares_init.c:ares__generate_new_id() . I found that putting short r=0 + in this function (instead of short r) makes all Valgrind warnings go away. + I have studied ares__rc4() too, and this is the offending line: + + buffer_ptr[counter] ^= state[xorIndex]; (ares_query.c:62) + + This is what triggers Valgrind.. buffer_ptr is unitialised in this function, + and by applying ^= on it, it remains unitialised. Version 1.4.0 (June 8, 2007) -- cgit v1.2.3