netdev - Re: [PATCH net] Phonet: hold GPRS device while cleaning up

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20081208.022153.93642210.davem@davemloft.net>
Date:	Mon, 08 Dec 2008 02:21:53 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	remi.denis-courmont@...ia.com
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH net] Phonet: hold GPRS device while cleaning up

From: Rémi Denis-Courmont <remi.denis-courmont@...ia.com>
Date: Mon,  8 Dec 2008 12:09:25 +0200

This is just a side-comment about the phonet stack, not directly
related to this patch.

> diff --git a/net/phonet/pep-gprs.c b/net/phonet/pep-gprs.c
> index 9978afb..9602bfc 100644
> --- a/net/phonet/pep-gprs.c
> +++ b/net/phonet/pep-gprs.c
> @@ -340,8 +340,10 @@ void gprs_detach(struct sock *sk)
>  	release_sock(sk);
>  
>  	printk(KERN_DEBUG"%s: detached\n", net->name);
> +	dev_hold(net); /* TX work still needs it */
>  	unregister_netdev(net);
>  	flush_scheduled_work();
> +	dev_put(net);
>  	sock_put(sk);
>  	skb_queue_purge(&dev->tx_queue);
>  }

Can we get the phonet stack using more sensible local
variable names for netdev pointers?

We have network namespace objects, and everything says
"net" as the variable name to hold pointers to such
objects.

Using the same local variable name for "struct netdev"
pointers breeds confusion and makes phonet patches and
code difficult to read for people familiar with the
rest of the Linux networking.

Since you use "dev", which is the typical way to name
netdev pointers in networking code, for the phonet et al.
private stuff, one thing you can do is adopt the technique
I usually use:

1) Driver/subsystem private objects get a two letter local
   variable name, the second letter is 'p' for pointer.
   So a TG3 driver private pointer variable is "tp".  You'll
   see this in pretty much every single driver I ever wrote.

2) struct netdev pointer variables are "dev"

3) Network namespace pointer variables are "net"

With this we'll achieve consistency and much easier to read
and validate patches.

Just a suggestion :)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html