Re: [PATCH net-next v3 1/3] net: ipv6: Unduplicate {raw,udp}v6_sendmsg code

[Lorenzo Colitti <lorenzo@google.com>]

On Wed, Apr 23, 2014 at 12:48 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
>> +     *optp = NULL;
>> +     *dstp = NULL;
>> +     *hlimit = *tclass = *dontfrag = -1;
>
> Do we need those? If we return != 0 from ip6_datagram_send_common we
> know the in/out arguments may or maybe got an update an as such cannot
> use them.
>
> Caller can initialize dstp = NULL so we can pass dstp to dst_release in the
> error path in the caller function (dst_release checks for argument != NULL).

Initalizing optp was not useful. As you say, the callers won't use it
if we return an error. I've removed it.

I've also removed the initialization of dstp. After thinking about it,
I agree it's bad for the callers to rely on this function to set dstp
to NULL on failure. It's more reasonable for the callers to assume
that the dst pointer they pass in won't be touched on failure.

As for hlimit, tclass and dontfrag, they (and connected), I think it's
useful to initialize them here because it simplifies the code. Since
we set them to -1 early on, we can later do things like "if (*hlimit <
0) { set the default hop limit }" and have things work regardless of
whether a hop limit was passed in via cmsg. I don't think it's
confusing: variables are all calculated and returned by this function,
and the callers should not be setting them since they have no
information on what to set them to. Also, they don't need to be freed.
I've moved them closer to where they are actually set.

>> +     if (!ipv6_addr_any(daddr))
>> +             fl6->daddr = *daddr;
>> +     else
>> +             fl6->daddr.s6_addr[15] = 0x1; /* :: means loopback (BSD'ism) */
>
> Maybe a newline?

Added.

> We would also need a fl6_sock_release here because we finished processing
> it.
>
> You could also release the flowlabel after fl6_merge_options and could remove
> the fl6_sock_release after ip6_sk_dst_lookup_flow in the error handling.

Yes, that's better. Done.