Re: ifioctl() cleanup, step 2

2017-10-11 Thread Alexander Bluhm
On Wed, Oct 11, 2017 at 10:44:22AM +0200, Martin Pieuchot wrote: > Moar cleanup to be able to selectively take the NET_LOCK() around some > ioctls. > > This diff change many "return (error)" into "break". It looks a bit inconsistent, where you replace the return and where not. But I am sure

Re: ifioctl() cleanup, step 2

2017-10-11 Thread Alexandr Nedvedicky
On Wed, Oct 11, 2017 at 12:43:45PM +0200, Martin Pieuchot wrote: > On 11/10/17(Wed) 12:28, Alexandr Nedvedicky wrote: > > Hello, > > > > just for my curiosity: > > > > why do you leave some returns untreated? is that intentional? > > Yes it is intentional to make the diff shorter and easier

Re: ifioctl() cleanup, step 2

2017-10-11 Thread Martin Pieuchot
On 11/10/17(Wed) 12:28, Alexandr Nedvedicky wrote: > Hello, > > just for my curiosity: > > why do you leave some returns untreated? is that intentional? Yes it is intentional to make the diff shorter and easier to review. > just like here: > @@ -2048,8 +2048,11 @@ ifioctl(struct

Re: ifioctl() cleanup, step 2

2017-10-11 Thread Alexandr Nedvedicky
Hello, just for my curiosity: why do you leave some returns untreated? is that intentional? just like here: @@ -2048,8 +2048,11 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) case SIOCGVNETID: case SIOCGIFPAIR: case

ifioctl() cleanup, step 2

2017-10-11 Thread Martin Pieuchot
Moar cleanup to be able to selectively take the NET_LOCK() around some ioctls. This diff change many "return (error)" into "break". It adds error checks for SIOC{A,D}IFGROUP. The only driver handling these ioctl(2)s is... carp(4)! ok? Index: net/if.c

Re: ifioctl() cleanup

2017-10-10 Thread Alexander Bluhm
On Tue, Oct 10, 2017 at 03:55:30PM +0200, Martin Pieuchot wrote: > Similar diff without factorizing privilege checks, deraadt@ pointed out > it is too fragile. > > ok? OK bluhm@ > > Index: net/if.c > === > RCS file:

Re: ifioctl() cleanup

2017-10-10 Thread Martin Pieuchot
On 09/10/17(Mon) 16:34, Martin Pieuchot wrote: > Here's a small cleanup to make it easy to push the NET_LOCK() further > down. > > Diff below factorize all ioctl requests need root privileges in one > switch () block. > > It moves SIOCIFAFATTACH/SIOCIFAFDETACH where it belongs, to the block >

Re: ifioctl() cleanup

2017-10-10 Thread Michele Curti
On Mon, Oct 09, 2017 at 04:34:38PM +0200, Martin Pieuchot wrote: > Here's a small cleanup to make it easy to push the NET_LOCK() further > down. > > Diff below factorize all ioctl requests need root privileges in one > switch () block. > > It moves SIOCIFAFATTACH/SIOCIFAFDETACH where it belongs,

ifioctl() cleanup

2017-10-09 Thread Martin Pieuchot
Here's a small cleanup to make it easy to push the NET_LOCK() further down. Diff below factorize all ioctl requests need root privileges in one switch () block. It moves SIOCIFAFATTACH/SIOCIFAFDETACH where it belongs, to the block that keeps track of the timestamps of the last change. It gets