Re: [PATCH 07/39] wimax: generic WiMAX device management (registration, deregistration, etc)

Patrick McHardy
2008-11-27T10:40:58Z

Inaky Perez-Gonzalez wrote:
> +static
> +struct nla_policy wimax_gnl_result_policy[WIMAX_GNL_ATTR_MAX + 1] = {

const please for all policies.

> + // This is really a signed-64 bit number that has to be
> + // casted from a u64
> + [WIMAX_GNL_RESULT_CODE] = { .type = NLA_U64 },
> +};
> +*/
> +

> +int wimax_gnl_send_rp_result(struct wimax_dev *wimax_dev,
> + struct genl_info *genl_info,
> + ssize_t code)
> +{
> + int result;
> + struct device *dev = wimax_dev_to_dev(wimax_dev);
> + void *data;
> + struct sk_buff *reply_skb;
> + s64 code_s64 = code;
> +
> + d_fnstart(3, dev, "(wimax_dev %p info %p code %zd)\n",
> + wimax_dev, genl_info, code);
> + result = -ENOMEM;
> + reply_skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + if (reply_skb == NULL)
> + goto error_new;
> + data = genlmsg_put_reply(reply_skb, genl_info,
> + &wimax_dev->gnl_family,
> + 0, WIMAX_GNL_RP_RESULT);
> + if (data == NULL)
> + goto error_put_reply;
> +
> + result = nla_put_u64(reply_skb, WIMAX_GNL_RESULT_CODE, (u64) code_s64);
> + if (result < 0)
> + dev_err(dev, "Error putting attribute: %d\n", result);

This is not how netlink protocols should treat errors.
It should return -ENOSPC (everywhere else also of course).

> +
> + genlmsg_end(reply_skb, data);
> +
> + result = genlmsg_reply(reply_skb, genl_info);
> + if (result < 0)
> + goto error_reply;
> + d_fnend(3, dev, "(wimax_dev %p info %p code %zd) = 0\n",
> + wimax_dev, genl_info, code);
> + return 0;
> +
> +error_reply:
> +error_put_reply:
> + nlmsg_free(reply_skb);
> +error_new:
> + d_fnend(3, dev, "(wimax_dev %p info %p code %zd) = %d\n",
> + wimax_dev, genl_info, code, result);
> + return result;
> +}