[ovs-dev] [PreQoS 10/13] netlink: Make nl_sock_transact() always return a reply on success.

[Justin Pettit]

Looks good.

--Justin


On May 27, 2010, at 1:33 PM, Ben Pfaff wrote:

> Until now, if nl_sock_transact() received a reply that merely acknowledged
> success, without providing any other payload, it would return success but
> not provide the reply to its caller.  This is inconsistent and could easily
> cause a segfault in a caller that expects to see the reply on success, if
> kernel behavior changed, for whatever reason, so that a request that
> previously returned data now just returns an acknowledgment.  In practice
> this kind of change should never happen, but it is still better to handle
> it properly.
> ---
> lib/netlink.c |    9 ++++++++-
> 1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/lib/netlink.c b/lib/netlink.c
> index 02bc3d8..73328e1 100644
> --- a/lib/netlink.c
> +++ b/lib/netlink.c
> @@ -421,7 +421,14 @@ recv:
>         ofpbuf_delete(reply);
>         goto recv;
>     }
> -    if (nl_msg_nlmsgerr(reply, &retval)) {
> +
> +    /* If the reply is an error, discard the reply and return the error code.
> +     *
> +     * Except: if the reply is just an acknowledgement (error code of 0), and
> +     * the caller is interested in the reply (replyp != NULL), pass the reply
> +     * up to the caller.  Otherwise the caller will get a return value of 0
> +     * and null '*replyp', which makes unwary callers likely to segfault. */
> +    if (nl_msg_nlmsgerr(reply, &retval) && (retval || !replyp)) {
>         ofpbuf_delete(reply);
>         if (retval) {
>             VLOG_DBG_RL(&rl, "received NAK error=%d (%s)",
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org