Jakub Hrozek wrote:
> Hello,
> thanks for reviewing the patch!
>
> On 07/01/2009 08:06 PM, Uli wrote:
>> Hi,
>
>> This looks like it will break badly if those fields aren't 2 byte large (they
>> aren't)
>
> My main source of documentation when writing the function was RFC 2782
> [1], which specifically says "This is a 16 bit unsigned integer in
> network byte order." in description of the port, weight and priority
> fields on pages 2 and 3 of the RFC. I also checked the source of ISC
> Bind 9.6.1, the structure for SRV records is declared in file
> lib/dns/rdata/in_1/srv_33.h and the fields are declared as isc_uint16_t
> there.
>
> Actually, I changed the weight, priority and port members of struct
> srv_reply to be u_int16_t as they were plain int in the previous patch.
> New version of the patch is attached.
Ok, this should fix my doubts. :)
> (Related question: would you prefer using u_int_XX_t from sys/types.h
> which is unconditionally included in ares.h or rather ISO C99-compliant
> uint_XX_t types from stdint.h?)
Don't ask me, I'm not a regular here.
>> and if the mem isn't initialized to zero (it isn't, allocated via malloc
>> which doesn't initialize mem).
>
> I don't quite understand why is this a problem. The uninitialized value
> of both bytes is overwritten with the memcpy call.
Yeah, but before you had only initialized the lower 2 bytes of a e.g. 4 byte
large integer which meant memcpy() didn't completely overwrite it.
>
>> Oh and I'd expect this to break badly on some endian where it writes to the high
>> bytes of the "int"s.
>
>
> I expected the bytes in the ares payload (aptr) to be in the network
> byte order and that's why I converted them to host byte order with
> ntohs. I'm not sure what's wrong with this. Can you elaborate, please?
With "this" I meant again the memcpy() call.
In case it wasn't clear (sorry!): My only complaint was that you used memcpy()
to initialize only part of a larger integer variable. If you use some *int16_t
variable this is fixed.
Uli
-- "Do you know that books smell like nutmeg or some spice from a foreign land?" -- Faber in Fahrenheit 451Received on 2009-07-02