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: <20090614183357.GE3639@local>
Date:	Sun, 14 Jun 2009 20:33:57 +0200
From:	"Hans J. Koch" <hjk@...utronix.de>
To:	Wolfram Sang <w.sang@...gutronix.de>
Cc:	"Hans J. Koch" <hjk@...utronix.de>, linux-kernel@...r.kernel.org,
	linuxppc-dev@...abs.org, devicetree-discuss@...abs.org,
	Magnus Damm <magnus.damm@...il.com>, Greg KH <gregkh@...e.de>
Subject: Re: [PATCH 2/2] uio: add an of_genirq driver

On Sun, Jun 14, 2009 at 07:14:06PM +0200, Wolfram Sang wrote:
> Hello Hans,
> 
> > > +	uioinfo->irq = irq_of_parse_and_map(op->node, 0);
> > > +	if (!uioinfo->irq)
> > > +		uioinfo->irq = UIO_IRQ_NONE;
> > 
> > Please don't do this. It's inconsistent if all other UIO drivers require
> > people to use UIO_IRQ_NONE and you also allow zero. UIO_IRQ_NONE was
> > introduced because 0 may be a legal interrupt number on some platforms.
> 
> Yes, well, the '0' vs. 'NO_IRQ' thing is still not fully sorted out AFAIK.

A "cat /proc/interrupts" on any common x86 PC shows you that IRQ 0 is used
there. OK, it's unlikely someone wants to write a UIO driver for that one,
but that might be different on other platforms.
Anyway, 0 is a valid IRQ number, so it cannot be used as "no irq".

> But you are possibly right here, as long as irq_of_parse_and_map does return
> NO_IRQ, I should explicitly check for it, like this:
> 
> 	if (uioinfo->irq == NO_IRQ)
> 		uioinfo->irq = UIO_IRQ_NONE;

Sorry for my ignorance, but what is NO_IRQ? If I do a

grep -r NO_IRQ include/

I get nothing.

> 
> > > +/* Match table for of_platform binding */
> > > +static const struct of_device_id __devinitconst uio_of_genirq_match[] = {
> > 
> > checkpatch.pl complains about that. Please check.
> 
> Did that, it is a false positive. See here:
> 
> http://lkml.indiana.edu/hypermail/linux/kernel/0906.1/02780.html

Well, you claim it's a false positive. So far, you did not get any responses,
AFAICS. I tend to agree with you, but I'd like to avoid patches that don't
pass checkpatch.pl, whatever the reason. Either the false positive gets
confirmed and fixed, or you should fix your patch.

Thanks,
Hans

> 
> Regards,
> 
>    Wolfram
> 
> -- 
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |


--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ