[PATCH v2] iio: core: Replace BUG() with WARN_ON_ONCE() and error return

Rishab Madhugiri posted 1 patch 1 month ago
drivers/iio/industrialio-core.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
[PATCH v2] iio: core: Replace BUG() with WARN_ON_ONCE() and error return
Posted by Rishab Madhugiri 1 month ago
As documented in Documentation/process/deprecated.rst, the
use of BUG() and BUG_ON() should be avoided as they could
cause a complete system crash, preventing further debugging.

In iio_get_time_ns() and current_timestamp_clock_show(), the
clock type is already validated against supported clock
types prior to these calls, making the default switch
branches expected to be unreachable. Replace these with
WARN_ON_ONCE(1) followed by an error return to avoid a
crash and to log the highly unexpected condition.

Signed-off-by: Rishab Madhugiri <rishab.madhugiri@gmail.com>
---
v2:
 - Use WARN_ON_ONCE(1) instead of WARN_ONCE() with string message as suggested
   by Jonathan Cameron.
 - Reference Documentation/process/deprecated.rst and clarify unreachability
   of default branches and reasoning for using WARN family over BUG family 
   in commit message as per Andy Shevchenko's feedback.

 drivers/iio/industrialio-core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 767a7794624a..819a864d1136 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -341,7 +341,8 @@ s64 iio_get_time_ns(const struct iio_dev *indio_dev)
 	case CLOCK_TAI:
 		return ktime_get_clocktai_ns();
 	default:
-		BUG();
+		WARN_ON_ONCE(1);
+		return 0;
 	}
 }
 EXPORT_SYMBOL(iio_get_time_ns);
@@ -1518,7 +1519,8 @@ static ssize_t current_timestamp_clock_show(struct device *dev,
 	case CLOCK_TAI:
 		break;
 	default:
-		BUG();
+		WARN_ON_ONCE(1);
+		return -EINVAL;
 	}
 
 	return sysfs_emit(buf, "%s\n", clock_names[clk]);
-- 
2.43.0
Re: [PATCH v2] iio: core: Replace BUG() with WARN_ON_ONCE() and error return
Posted by Andy Shevchenko 1 month ago
On Wed, Aug 26, 2026 at 09:31:40PM +0000, Rishab Madhugiri wrote:
> As documented in Documentation/process/deprecated.rst, the
> use of BUG() and BUG_ON() should be avoided as they could
> cause a complete system crash, preventing further debugging.

Right, but deprecation doesn't mean the _old_ existing code must be immediately
changed. I leave it to Jonathan, but I see no value in this change except
the unneeded churn. If we ever want to drop BUG(), it should be done tree wide.
Hence no tag given.

> In iio_get_time_ns() and current_timestamp_clock_show(), the
> clock type is already validated against supported clock
> types prior to these calls, making the default switch
> branches expected to be unreachable. Replace these with
> WARN_ON_ONCE(1) followed by an error return to avoid a
> crash and to log the highly unexpected condition.

There is like 10+ character space on each line, the commit message shouldn't
be that condensed.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v2] iio: core: Replace BUG() with WARN_ON_ONCE() and error return
Posted by Rishab M 1 month ago
Thanks for the feedback.  I totally understand the perspective on code churn
versus making tree wide cleanups, and I will let Jonathan make the final call
on whether this is suitable.  I've also noted the wrapping width feedback for
commit messages in the future.

Best,
Rishab

On Wed, Aug 26, 2026 at 11:41 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Wed, Aug 26, 2026 at 09:31:40PM +0000, Rishab Madhugiri wrote:
> > As documented in Documentation/process/deprecated.rst, the
> > use of BUG() and BUG_ON() should be avoided as they could
> > cause a complete system crash, preventing further debugging.
>
> Right, but deprecation doesn't mean the _old_ existing code must be immediately
> changed. I leave it to Jonathan, but I see no value in this change except
> the unneeded churn. If we ever want to drop BUG(), it should be done tree wide.
> Hence no tag given.
>
> > In iio_get_time_ns() and current_timestamp_clock_show(), the
> > clock type is already validated against supported clock
> > types prior to these calls, making the default switch
> > branches expected to be unreachable. Replace these with
> > WARN_ON_ONCE(1) followed by an error return to avoid a
> > crash and to log the highly unexpected condition.
>
> There is like 10+ character space on each line, the commit message shouldn't
> be that condensed.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Re: [PATCH v2] iio: core: Replace BUG() with WARN_ON_ONCE() and error return
Posted by Jonathan Cameron 4 weeks ago
On Thu, 27 Aug 2026 15:24:52 -0700
Rishab M <rishab.madhugiri@gmail.com> wrote:

> Thanks for the feedback.  I totally understand the perspective on code churn
> versus making tree wide cleanups, and I will let Jonathan make the final call
> on whether this is suitable.  I've also noted the wrapping width feedback for
> commit messages in the future.
> 
Please comment inline.

Anyhow, I have applied this but mostly because it is easier for cases like
this to do so than to reply to a series of folk proposing the change.

Slightly rubbish reason, but meh, too many patches floating around and this
is a harmless way to potentially cut that down a little in the longer term.

Thanks,

Jonathan

> Best,
> Rishab
> 
> On Wed, Aug 26, 2026 at 11:41 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> >
> > On Wed, Aug 26, 2026 at 09:31:40PM +0000, Rishab Madhugiri wrote:  
> > > As documented in Documentation/process/deprecated.rst, the
> > > use of BUG() and BUG_ON() should be avoided as they could
> > > cause a complete system crash, preventing further debugging.  
> >
> > Right, but deprecation doesn't mean the _old_ existing code must be immediately
> > changed. I leave it to Jonathan, but I see no value in this change except
> > the unneeded churn. If we ever want to drop BUG(), it should be done tree wide.
> > Hence no tag given.
> >  
> > > In iio_get_time_ns() and current_timestamp_clock_show(), the
> > > clock type is already validated against supported clock
> > > types prior to these calls, making the default switch
> > > branches expected to be unreachable. Replace these with
> > > WARN_ON_ONCE(1) followed by an error return to avoid a
> > > crash and to log the highly unexpected condition.  
> >
> > There is like 10+ character space on each line, the commit message shouldn't
> > be that condensed.
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> >