[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20101120183007.GH13356@enneenne.com>
Date: Sat, 20 Nov 2010 19:30:07 +0100
From: Rodolfo Giometti <giometti@...eenne.com>
To: Joe Perches <joe@...ches.com>
Cc: Alexander Gordeev <lasaine@....cs.msu.su>,
linux-kernel@...r.kernel.org,
"Nikita V. Youshchenko" <yoush@...msu.su>,
linuxpps@...enneenne.com, Tejun Heo <tj@...nel.org>
Subject: Re: [PATCHv4 10/17] pps: use BUG_ON for kernel API safety checks
On Sat, Nov 20, 2010 at 09:01:41AM -0800, Joe Perches wrote:
> > > - if ((event & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) == 0) {
> > > - dev_err(pps->dev, "unknown event (%x)\n", event);
> > > - return;
> > > - }
> > > + /* check event type */
> > > + BUG_ON((event & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) == 0);
> >
> > Ack.
> >
> > This is a correct usage of BUG_ON. :)
>
> I don't think that's true.
>
> /*
> * Don't use BUG() or BUG_ON() unless there's really no way out; one
> * example might be detecting data structure corruption in the middle
> * of an operation that can't be backed out of. If the (sub)system
> * can somehow continue operating, perhaps with reduced functionality,
> * it's probably not BUG-worthy.
> *
> * If you're tempted to BUG(), think again: is completely giving up
> * really the *only* solution? There are usually better options, where
> * users don't need to reboot ASAP and can mostly shut down cleanly.
> */
If (event & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) == 0 then PPS's
data has been corrupted. The PPS subsystem doesn't work correctly even
if the whole system still continues running.
However I already have not acknowledged the patch.
Ciao,
Rodolfo
--
GNU/Linux Solutions e-mail: giometti@...eenne.com
Linux Device Driver giometti@...ux.it
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it
--
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