Re: [PATCH] ext4: remove unreachable statement inside __es_insert_extent()

[Theodore Y. Ts'o]

On Thu, Aug 22, 2019 at 03:37:43PM +0900, Austin Kim wrote:
> __es_insert_extent() never returns -EINVAL after BUG is executed.
> So remove unreachable code.
> ---
>  fs/ext4/extents_status.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c
> index a959adc..7f97360 100644
> --- a/fs/ext4/extents_status.c
> +++ b/fs/ext4/extents_status.c
> @@ -781,7 +781,6 @@ static int __es_insert_extent(struct inode *inode, struct extent_status *newes)
>  			p = &(*p)->rb_right;
>  		} else {
>  			BUG();
> -			return -EINVAL;

This would not be safe in the case of !CONFIG_BUG.  (See init/Kconfig)

It's fair to argue that we shouldn't have CONFIG_BUG --- or
!CONFIG_BUG should still cause the kernel to stop without actually
printing the full BUG information, for those tiny kernel applications
which are really worried about kernel text space.

It also would be fair to argue that we should remove the unreachable
annotation for BUG(), or even, add a *reachable* annotation to catch
code where something something terribly might happen if the kernel is
built with !CONFIG_BUG and we trip against a bug.

But this is a much higher level issue than your sending individual
paches subsystems.

Regards,

						- Ted