netdev - Re: [PATCH V8 08/13] posix clocks: cleanup the CLOCK

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]

Date:	Wed, 12 Jan 2011 08:37:28 +0100
From:	Richard Cochran <richardcochran@...il.com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	linux-kernel@...r.kernel.org, linux-api@...r.kernel.org,
	netdev@...r.kernel.org, Alan Cox <alan@...rguk.ukuu.org.uk>,
	Arnd Bergmann <arnd@...db.de>,
	Christoph Lameter <cl@...ux.com>,
	David Miller <davem@...emloft.net>,
	John Stultz <johnstul@...ibm.com>,
	Krzysztof Halasa <khc@...waw.pl>,
	Peter Zijlstra <peterz@...radead.org>,
	Rodolfo Giometti <giometti@...ux.it>
Subject: Re: [PATCH V8 08/13] posix clocks: cleanup the CLOCK_DISPTACH macro

On Tue, Jan 11, 2011 at 01:57:23PM +0100, Thomas Gleixner wrote:
> I wonder whether we should be a bit more clever here and create an
> extra entry for posix_cpu_timers in the posix_clocks array and do the
> following:
...
> That reduces the code significantly and the MAX_CLOCKS check in
> clock_get_array_id() replaces the invalid_clock() check in the
> syscalls as well. It does not matter whether we check this before or
> after copying stuff from user.

Well, this does reduce the number of LOC, but the number of
comparisons is the same. I think the code size would be also no
different.

> Adding your new stuff requires just another entry in the array, the
> setup of the function pointers and the CLOCKFD check in
> clock_get_array_id(). Everything else just falls in place.

For me, I am not sure if either version is really very pretty or easy
to understand.

My instinct is to keep the posix_cpu_X and pc_X functions out of the
array of static clock id functions, if only to make the distinction
between the legacy static ids and new dynamic ids clear.

The conclusion from the "dynamic clock as file" discussion was that we
don't want to add any more static clock ids, and new clocks should use
the new, dynamic clock API. So, we should discourage any future
attempt to add to that function array!

Having said that, if you insist on it, I won't mind reworking the
dispatch code as you suggested.

> > +
> > +#define CLOCK_DISPATCH(clock, call, arglist) dispatch_##call arglist
> > +
> 
> Can we get rid of this completely please ?

Yes, gladly.

> >  clock_nanosleep_restart(struct restart_block *restart_block)
> >  {
> > -	clockid_t which_clock = restart_block->arg0;
> > -
> 
> How does that compile ?

Because the CLOCK_DISPATCH macro, above, makes no use of the first
argument! I have removed the macro for the next round.

> >  	return CLOCK_DISPATCH(which_clock, nsleep_restart,
> >  			      (restart_block));
> >  }

Thanks,
Richard
--
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