On Thursday 27 November 2008, Patrick McHardy wrote:

Inaky Perez-Gonzalez wrote:

quoted

+static
+struct nla_policy wimax_gnl_result_policy[WIMAX_GNL_ATTR_MAX + 1] = {

const please for all policies.

Done.

quoted

+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).

Other than the fact that this function is gone after Johannes comments
(so for everywhere else), wouldn't returning the -EMSGSIZE nla_put_*() 
returns enough?

I changed all of them to be something like:

result = nla_put*();
if (result < 0)
	goto error path /* release skb/msg, return result */

Thanks,

-- 
Inaky