 053c095a82
			
		
	
	
	053c095a82
	
	
	
		
			
			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);
 | |
| }
 | |
| 
 |