linux-kernel - Re: random.c changes for sparse irq

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:	Thu, 01 Jan 2009 03:31:56 -0600
From:	Matt Mackall <mpm@...enic.com>
To:	Yinghai Lu <yinghai@...nel.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linus Torvalds <torvalds@...l.org>, Ingo Molnar <mingo@...e.hu>
Subject: Re: random.c changes for sparse irq_desc are crap

On Wed, 2008-12-31 at 16:14 -0800, Yinghai Lu wrote:
> On Wed, Dec 31, 2008 at 3:40 PM, Matt Mackall <mpm@...enic.com> wrote:
> > On Wed, 2008-12-31 at 15:07 -0800, Yinghai Lu wrote:
> >> Matt Mackall wrote:
> >> > I just noticed you merged a change that pointlessly converts two
> >> > random.c functions into ugly random.h inlines without going through the
> >> > maintainer.
> >> >
> >> > I also don't like the look of the newly-introduced sparse variants of
> >> > these functions. Failure to find an irq descriptor in
> >> > get_timer_rand_state is a BUG_ON should-never-happen sort of condition,
> >> > not something to silently ignore. Letting the code try to dereference
> >> > NULL is preferred here: we'll actually be able to find and fix the
> >> > broken driver that's throwing around meaningless irq vectors.
> >> >
> >> > Throwing away the timer_state pointer in the set_timer_rand_state
> >> > function is similarly bogus in addition to being a memory leak.
> >> >
> >> > Please fix this up.
> >> >
> >>
> >> want something like this?
> >
> > Not quite.
> >
> > First, please turn these back into normal functions in random.c.
> > Inlining functions is generally discouraged these days unless you have a
> > good reason and numbers to back it up.
> 
> Ingo wanted to hide that #ifdef to .h

Ahh. I think that makes sense in some situations, but I'd prefer not to
do that here. Some headers are actually meant to be more readable than
the corresponding .c file. Also, we still end up with an ifdef in the .c
file so we're not actually winning.

-- 
Mathematics is the supreme nostalgia of our time.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/