Code review comment for lp:~martin-lp/hipl/hipl_retransmissions

Hi,

On 22/12/11 00:22, Diego Biurrun wrote:
> On Wed, Dec 21, 2011 at 11:32:26AM +0100, Diego Biurrun wrote:
>> On Tue, Dec 20, 2011 at 11:43:26AM +0000, David Martin wrote:
>>> David Martin has proposed merging lp:~martin-lp/hipl/hipl_retransmissions into lp:hipl.
>>>
>>> --- hipd/output.c 2011-11-10 10:53:21 +0000
>>> +++ hipd/output.c 2011-12-20 11:41:33 +0000
>>> @@ -1118,22 +1118,25 @@
>>> const struct hip_common *msg,
>>> struct hip_hadb_state *entry)
>>> {
>>> - int len = hip_get_msg_total_len(msg);
>>> + int len = hip_get_msg_total_len(msg);
>>> + struct hip_msg_retrans *retrans;
>>>
>>> if (!entry) {
>>> return 0;
>>> }
>>>
>>> + retrans =&entry->hip_msg_retrans[entry->next_retrans_slot];
>>> +
>>> /* Not reusing the old entry as the new packet may have different length */
>>> + memset(retrans->buf, 0, HIP_MAX_NETWORK_PACKET);
>>
>> see above
>>
>>> + memcpy(retrans->buf, msg, len);
>>> + memcpy(&retrans->saddr, src_addr, sizeof(struct in6_addr));
>>> + memcpy(&retrans->daddr, peer_addr, sizeof(struct in6_addr));
>
> The comment I intended to add here was that this looks as if it can be
> done by assignments instead of memcpy.

if you refer to IPv6 addresses, they have to be copied (no such thing as
128-bit integer). Anyway, the most elegant way is to use
ipv6_addr_copy() instead of memcpy().