From: Rodolfo Giometti <giometti@enneenne.com>
To: Joe Perches <joe@perches.com>
Cc: Alexander Gordeev <lasaine@lvk.cs.msu.su>,
linux-kernel@vger.kernel.org,
"Nikita V. Youshchenko" <yoush@cs.msu.su>,
linuxpps@ml.enneenne.com, Tejun Heo <tj@kernel.org>
Subject: Re: [PATCHv4 10/17] pps: use BUG_ON for kernel API safety checks
Date: Sat, 20 Nov 2010 19:30:07 +0100 [thread overview]
Message-ID: <20101120183007.GH13356@enneenne.com> (raw)
In-Reply-To: <1290272501.27951.27.camel@Joe-Laptop>
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@enneenne.com
Linux Device Driver giometti@linux.it
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - www.consulenti-ict.it
next prev parent reply other threads:[~2010-11-20 18:30 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-11-18 16:00 [PATCHv4 00/17] pps: several fixes and improvements Alexander Gordeev
2010-11-18 16:00 ` [PATCHv4 01/17] pps: trivial fixes Alexander Gordeev
2010-11-18 16:00 ` [PATCHv4 02/17] pps: declare variables where they are used in switch Alexander Gordeev
2010-11-18 16:00 ` [PATCHv4 03/17] pps: fix race in PPS_FETCH handler Alexander Gordeev
2010-11-20 15:20 ` Rodolfo Giometti
2010-11-18 16:00 ` [PATCHv4 04/17] pps: unify timestamp gathering Alexander Gordeev
2010-11-18 16:00 ` [PATCHv4 05/17] pps: access pps device by direct pointer Alexander Gordeev
2010-11-20 15:44 ` Rodolfo Giometti
2010-11-20 22:33 ` Alexander Gordeev
2010-11-21 8:26 ` Rodolfo Giometti
2010-11-22 15:01 ` Alexander Gordeev
2010-11-21 14:12 ` Alan Cox
2010-11-22 14:55 ` Alexander Gordeev
2010-11-18 16:00 ` [PATCHv4 06/17] pps: convert printk/pr_* to dev_* Alexander Gordeev
2010-11-20 15:49 ` Rodolfo Giometti
2010-11-20 21:33 ` Alexander Gordeev
2010-11-20 21:42 ` Joe Perches
2010-11-20 22:38 ` Alexander Gordeev
2010-11-21 8:19 ` Rodolfo Giometti
2010-11-18 16:01 ` [PATCHv4 07/17] pps: move idr stuff to pps.c Alexander Gordeev
2010-11-20 15:51 ` Rodolfo Giometti
2010-11-18 16:01 ` [PATCHv4 08/17] pps: add async PPS event handler Alexander Gordeev
2010-11-20 16:08 ` Rodolfo Giometti
2010-11-20 23:23 ` Alexander Gordeev
2010-11-21 8:37 ` Rodolfo Giometti
2010-11-18 16:01 ` [PATCHv4 09/17] pps: don't disable interrupts when using spin locks Alexander Gordeev
2010-11-20 16:09 ` Rodolfo Giometti
2010-11-18 16:01 ` [PATCHv4 10/17] pps: use BUG_ON for kernel API safety checks Alexander Gordeev
2010-11-20 16:13 ` Rodolfo Giometti
2010-11-20 17:01 ` Joe Perches
2010-11-20 18:30 ` Rodolfo Giometti [this message]
2010-11-21 0:40 ` Alexander Gordeev
2010-11-21 1:18 ` Joe Perches
2010-11-21 8:42 ` Rodolfo Giometti
2010-11-21 0:13 ` Alexander Gordeev
2010-11-21 8:41 ` Rodolfo Giometti
2010-11-18 16:01 ` [PATCHv4 11/17] pps: simplify conditions a bit Alexander Gordeev
2010-11-20 16:15 ` Rodolfo Giometti
2010-11-18 16:01 ` [PATCHv4 12/17] pps: timestamp is always passed to dcd_change() Alexander Gordeev
2010-11-20 16:23 ` Rodolfo Giometti
2010-11-21 0:44 ` Alexander Gordeev
2010-11-21 8:42 ` Rodolfo Giometti
2010-11-18 16:01 ` [PATCHv4 13/17] ntp: add hardpps implementation Alexander Gordeev
2010-11-20 16:27 ` Rodolfo Giometti
2010-11-21 1:05 ` Alexander Gordeev
2010-11-18 16:01 ` [PATCHv4 14/17] pps: capture MONOTONIC_RAW timestamps as well Alexander Gordeev
2010-11-18 19:42 ` john stultz
2010-11-18 16:01 ` [PATCHv4 15/17] pps: add kernel consumer support Alexander Gordeev
2010-11-18 16:01 ` [PATCHv4 16/17] pps: add parallel port PPS client Alexander Gordeev
2010-11-18 16:01 ` [PATCHv4 17/17] pps: add parallel port PPS signal generator Alexander Gordeev
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=20101120183007.GH13356@enneenne.com \
--to=giometti@enneenne.com \
--cc=joe@perches.com \
--cc=lasaine@lvk.cs.msu.su \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxpps@ml.enneenne.com \
--cc=tj@kernel.org \
--cc=yoush@cs.msu.su \
/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.