Contrary to common expectations for an "int" return, these functions
return only a positive value -- if used correctly they cannot even
return 0 because the message header will necessarily be in the skb.
This makes the very common pattern of
  if (genlmsg_end(...) < 0) { ... }
be a whole bunch of dead code. Many places also simply do
  return nlmsg_end(...);
and the caller is expected to deal with it.
This also commonly (at least for me) causes errors, because it is very
common to write
  if (my_function(...))
    /* error condition */
and if my_function() does "return nlmsg_end()" this is of course wrong.
Additionally, there's not a single place in the kernel that actually
needs the message length returned, and if anyone needs it later then
it'll be very easy to just use skb->len there.
Remove this, and make the functions void. This removes a bunch of dead
code as described above. The patch adds lines because I did
-	return nlmsg_end(...);
+	nlmsg_end(...);
+	return 0;
I could have preserved all the function's return values by returning
skb->len, but instead I've audited all the places calling the affected
functions and found that none cared. A few places actually compared
the return value with <= 0 in dump functionality, but that could just
be changed to < 0 with no change in behaviour, so I opted for the more
efficient version.
One instance of the error I've made numerous times now is also present
in net/phonet/pn_netlink.c in the route_dumpit() function - it didn't
check for <0 or <=0 and thus broke out of the loop every single time.
I've preserved this since it will (I think) have caused the messages to
userspace to be formatted differently with just a single message for
every SKB returned to userspace. It's possible that this isn't needed
for the tools that actually use this, but I don't even know what they
are so couldn't test that changing this behaviour would be acceptable.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
		
	
			
		
			
				
	
	
		
			136 lines
		
	
	
	
		
			2.8 KiB
			
		
	
	
	
		
			C
		
	
	
	
	
	
			
		
		
	
	
			136 lines
		
	
	
	
		
			2.8 KiB
			
		
	
	
	
		
			C
		
	
	
	
	
	
/*
 | 
						|
 * Copyright (C) 2007 Red Hat, Inc.  All rights reserved.
 | 
						|
 *
 | 
						|
 * This copyrighted material is made available to anyone wishing to use,
 | 
						|
 * modify, copy, or redistribute it subject to the terms and conditions
 | 
						|
 * of the GNU General Public License v.2.
 | 
						|
 */
 | 
						|
 | 
						|
#include <net/genetlink.h>
 | 
						|
#include <linux/dlm.h>
 | 
						|
#include <linux/dlm_netlink.h>
 | 
						|
#include <linux/gfp.h>
 | 
						|
 | 
						|
#include "dlm_internal.h"
 | 
						|
 | 
						|
static uint32_t dlm_nl_seqnum;
 | 
						|
static uint32_t listener_nlportid;
 | 
						|
 | 
						|
static struct genl_family family = {
 | 
						|
	.id		= GENL_ID_GENERATE,
 | 
						|
	.name		= DLM_GENL_NAME,
 | 
						|
	.version	= DLM_GENL_VERSION,
 | 
						|
};
 | 
						|
 | 
						|
static int prepare_data(u8 cmd, struct sk_buff **skbp, size_t size)
 | 
						|
{
 | 
						|
	struct sk_buff *skb;
 | 
						|
	void *data;
 | 
						|
 | 
						|
	skb = genlmsg_new(size, GFP_NOFS);
 | 
						|
	if (!skb)
 | 
						|
		return -ENOMEM;
 | 
						|
 | 
						|
	/* add the message headers */
 | 
						|
	data = genlmsg_put(skb, 0, dlm_nl_seqnum++, &family, 0, cmd);
 | 
						|
	if (!data) {
 | 
						|
		nlmsg_free(skb);
 | 
						|
		return -EINVAL;
 | 
						|
	}
 | 
						|
 | 
						|
	*skbp = skb;
 | 
						|
	return 0;
 | 
						|
}
 | 
						|
 | 
						|
static struct dlm_lock_data *mk_data(struct sk_buff *skb)
 | 
						|
{
 | 
						|
	struct nlattr *ret;
 | 
						|
 | 
						|
	ret = nla_reserve(skb, DLM_TYPE_LOCK, sizeof(struct dlm_lock_data));
 | 
						|
	if (!ret)
 | 
						|
		return NULL;
 | 
						|
	return nla_data(ret);
 | 
						|
}
 | 
						|
 | 
						|
static int send_data(struct sk_buff *skb)
 | 
						|
{
 | 
						|
	struct genlmsghdr *genlhdr = nlmsg_data((struct nlmsghdr *)skb->data);
 | 
						|
	void *data = genlmsg_data(genlhdr);
 | 
						|
 | 
						|
	genlmsg_end(skb, data);
 | 
						|
 | 
						|
	return genlmsg_unicast(&init_net, skb, listener_nlportid);
 | 
						|
}
 | 
						|
 | 
						|
static int user_cmd(struct sk_buff *skb, struct genl_info *info)
 | 
						|
{
 | 
						|
	listener_nlportid = info->snd_portid;
 | 
						|
	printk("user_cmd nlpid %u\n", listener_nlportid);
 | 
						|
	return 0;
 | 
						|
}
 | 
						|
 | 
						|
static struct genl_ops dlm_nl_ops[] = {
 | 
						|
	{
 | 
						|
		.cmd	= DLM_CMD_HELLO,
 | 
						|
		.doit	= user_cmd,
 | 
						|
	},
 | 
						|
};
 | 
						|
 | 
						|
int __init dlm_netlink_init(void)
 | 
						|
{
 | 
						|
	return genl_register_family_with_ops(&family, dlm_nl_ops);
 | 
						|
}
 | 
						|
 | 
						|
void dlm_netlink_exit(void)
 | 
						|
{
 | 
						|
	genl_unregister_family(&family);
 | 
						|
}
 | 
						|
 | 
						|
static void fill_data(struct dlm_lock_data *data, struct dlm_lkb *lkb)
 | 
						|
{
 | 
						|
	struct dlm_rsb *r = lkb->lkb_resource;
 | 
						|
 | 
						|
	memset(data, 0, sizeof(struct dlm_lock_data));
 | 
						|
 | 
						|
	data->version = DLM_LOCK_DATA_VERSION;
 | 
						|
	data->nodeid = lkb->lkb_nodeid;
 | 
						|
	data->ownpid = lkb->lkb_ownpid;
 | 
						|
	data->id = lkb->lkb_id;
 | 
						|
	data->remid = lkb->lkb_remid;
 | 
						|
	data->status = lkb->lkb_status;
 | 
						|
	data->grmode = lkb->lkb_grmode;
 | 
						|
	data->rqmode = lkb->lkb_rqmode;
 | 
						|
	if (lkb->lkb_ua)
 | 
						|
		data->xid = lkb->lkb_ua->xid;
 | 
						|
	if (r) {
 | 
						|
		data->lockspace_id = r->res_ls->ls_global_id;
 | 
						|
		data->resource_namelen = r->res_length;
 | 
						|
		memcpy(data->resource_name, r->res_name, r->res_length);
 | 
						|
	}
 | 
						|
}
 | 
						|
 | 
						|
void dlm_timeout_warn(struct dlm_lkb *lkb)
 | 
						|
{
 | 
						|
	struct sk_buff *uninitialized_var(send_skb);
 | 
						|
	struct dlm_lock_data *data;
 | 
						|
	size_t size;
 | 
						|
	int rv;
 | 
						|
 | 
						|
	size = nla_total_size(sizeof(struct dlm_lock_data)) +
 | 
						|
	       nla_total_size(0); /* why this? */
 | 
						|
 | 
						|
	rv = prepare_data(DLM_CMD_TIMEOUT, &send_skb, size);
 | 
						|
	if (rv < 0)
 | 
						|
		return;
 | 
						|
 | 
						|
	data = mk_data(send_skb);
 | 
						|
	if (!data) {
 | 
						|
		nlmsg_free(send_skb);
 | 
						|
		return;
 | 
						|
	}
 | 
						|
 | 
						|
	fill_data(data, lkb);
 | 
						|
 | 
						|
	send_data(send_skb);
 | 
						|
}
 | 
						|
 |