Re: [PATCH] mm/mmap: replace if (cond) BUG() with BUG_ON()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 6 Jan 2021, Andrew Morton wrote:
> On Tue, 5 Jan 2021 20:28:27 -0800 (PST) Hugh Dickins <hughd@xxxxxxxxxx> wrote:
> 
> > Alex, please consider why the authors of these lines (whom you
> > did not Cc) chose to write them without BUG_ON(): it has always
> > been preferred practice to use BUG_ON() on predicates, but not on
> > functionally effective statements (sorry, I've forgotten the proper
> > term: I'd say statements with side-effects, but here they are not
> > just side-effects: they are their main purpose).
> > 
> > We prefer not to hide those away inside BUG macros
> 
> Should we change that?  I find BUG_ON(something_which_shouldnt_fail())
> to be quite natural and readable.

Fair enough.  Whereas my mind tends to filter out the BUG lines when
skimming code, knowing they can be skipped, not needing that effort
to pull out what's inside them.

Perhaps I'm a relic and everyone else is with you: I can only offer
my own preference, which until now was supported by kernel practice.

> 
> As are things like the existing
> 
> BUG_ON(mmap_read_trylock(mm));
> BUG_ON(wb_domain_init(&global_wb_domain, GFP_KERNEL));
> 
> etc.

People say "the exception proves the rule".  Perhaps we should invite a
shower of patches to change those?  (I'd prefer not, I'm no fan of churn.)

> 
> No strong opinion here, but is current mostly-practice really
> useful?

You've seen my vote.  Now let the games begin!

Hugh




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux