All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yinghai Lu <yinghai@kernel.org>
To: Matt Mackall <mpm@selenic.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@osdl.org>, Ingo Molnar <mingo@elte.hu>
Subject: Re: random.c changes for sparse irq_desc are crap
Date: Wed, 31 Dec 2008 15:07:52 -0800	[thread overview]
Message-ID: <495BFB48.8080506@kernel.org> (raw)
In-Reply-To: <1230748169.19620.132.camel@calx>

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?

---
 include/linux/random.h |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Index: linux-2.6/include/linux/random.h
===================================================================
--- linux-2.6.orig/include/linux/random.h
+++ linux-2.6/include/linux/random.h
@@ -75,8 +75,7 @@ static inline struct timer_rand_state *g
 
 	desc = irq_to_desc(irq);
 
-	if (!desc)
-		return NULL;
+	BUG_ON(!desc);
 
 	return desc->timer_rand_state;
 }
@@ -87,8 +86,7 @@ static inline void set_timer_rand_state(
 
 	desc = irq_to_desc(irq);
 
-	if (!desc)
-		return;
+	BUG_ON(!desc);
 
 	desc->timer_rand_state = state;
 }

  reply	other threads:[~2008-12-31 23:08 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-31 18:29 random.c changes for sparse irq_desc are crap Matt Mackall
2008-12-31 23:07 ` Yinghai Lu [this message]
2008-12-31 23:40   ` Matt Mackall
2009-01-01  0:14     ` Yinghai Lu
2009-01-01  9:31       ` Matt Mackall
2009-01-02 15:59         ` Ingo Molnar
2009-01-03  8:06           ` [PATCH] sparseirq: move set/get_timer_rand_state back to .c Yinghai Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=495BFB48.8080506@kernel.org \
    --to=yinghai@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mpm@selenic.com \
    --cc=torvalds@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.