Hi Jukka,
> >> diff --git a/src/dnsproxy.c b/src/dnsproxy.c
> >>
> >> +struct cache_entry {
> >> + GHashTable *cache;
> >
> > why do you have a cache hast table inside the cache entry?
>
> Hmm, it seems not to be needed, I will remove it.
I guessed something about a reverse hash table lookup, but could not
find it. Keep it simple.
> >> +static struct cache_entry *cache_check(struct server_data *server,
> >> + gpointer request)
> >> +{
> >> + uint16_t type;
> >> + unsigned int offset = 0;
> >> + unsigned char *dns_body = (unsigned char *)request + 12;
> >> + unsigned char *question =&dns_body[offset];
> >> + struct cache_entry *entry;
> >> +
> >> + offset = strlen((char *)question) + 1;
> >> + type = ntohs(*(uint16_t *)(&dns_body[offset]));
> >
> > Are we sure that we do not need unaligned access here?
>
> Good point. I did not see any problems with Atom but there might be
> alignment issues in other processors.
The Atom is still an x86 architecture. We are fine with any alignment
here. However PowerPC and others might not.
> >> +
> >> + memcpy(ptr, msg, 12);
> >> + ptr += 12;
> >> + memcpy(ptr, question, qlen);
> >> + ptr += qlen + 1;
> >> + *(uint16_t *)ptr = htons(type);
> >> + ptr += 2;
> >> + *(uint16_t *)ptr = htons(class);
> >> + ptr += 2;
> >> + memcpy(ptr, response, rsplen);
> >
> > If you copy data in the allocated buffer anyway, then setting it to zero
> > first is waste.
>
> I skipped the \0 at the end of the question (ptr += qlen + 1) because
> the buffer was set to 0. I can remove the zeroing, that is fine, I just
> need to remember to add the \0 in the buffer.
>
>
> > And no need to keep moving the pointer forward. Just use memcpy(ptr + ..
> > style operation.
>
> Sure.
>
>
> > Also what about unaligned access. Do we need to care?
>
> Probably it makes sense to try to play safe here, I will change that.
That said, we might wanna create some packed structs to allow us easier
access to DNS data packets.
Regards
Marcel
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman
