| Message ID | 1588126224-47877-1-git-send-email-zou_wei@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-next] blk-mq: Use BUG_ON() instead of BUG() | expand |
On Wed, Apr 29, 2020 at 10:10:24AM +0800, Zou Wei wrote: > Fixes coccicheck warning: > > block/blk-mq.c:546:2-5: WARNING: Use BUG_ON instead of if condition followed by BUG. > > Fixes: 63151a449eba ("blk-mq: allow drivers to hook into I/O completion") > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Zou Wei <zou_wei@huawei.com> > --- > block/blk-mq.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index bcc3a23..49a227e 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -542,8 +542,7 @@ EXPORT_SYMBOL(__blk_mq_end_request); > > void blk_mq_end_request(struct request *rq, blk_status_t error) > { > - if (blk_update_request(rq, error, blk_rq_bytes(rq))) > - BUG(); > + BUG_ON(blk_update_request(rq, error, blk_rq_bytes(rq))); I don't think hiding something that actually does do the work in a BUG_ON ever is a good style.
On 4/29/20 1:26 AM, Christoph Hellwig wrote: > On Wed, Apr 29, 2020 at 10:10:24AM +0800, Zou Wei wrote: >> Fixes coccicheck warning: >> >> block/blk-mq.c:546:2-5: WARNING: Use BUG_ON instead of if condition followed by BUG. >> >> Fixes: 63151a449eba ("blk-mq: allow drivers to hook into I/O completion") >> Reported-by: Hulk Robot <hulkci@huawei.com> >> Signed-off-by: Zou Wei <zou_wei@huawei.com> >> --- >> block/blk-mq.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/block/blk-mq.c b/block/blk-mq.c >> index bcc3a23..49a227e 100644 >> --- a/block/blk-mq.c >> +++ b/block/blk-mq.c >> @@ -542,8 +542,7 @@ EXPORT_SYMBOL(__blk_mq_end_request); >> >> void blk_mq_end_request(struct request *rq, blk_status_t error) >> { >> - if (blk_update_request(rq, error, blk_rq_bytes(rq))) >> - BUG(); >> + BUG_ON(blk_update_request(rq, error, blk_rq_bytes(rq))); > > I don't think hiding something that actually does do the work in a > BUG_ON ever is a good style. Agree, it's a lot less readable. And, not that we've ever done that, but also fragile in a lot of code bases where a non-debug build would turn off the BUG_ON() equivalent, and hence never call blk_update_request(). So not a good practice anywhere for statements that have side effects.
diff --git a/block/blk-mq.c b/block/blk-mq.c index bcc3a23..49a227e 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -542,8 +542,7 @@ EXPORT_SYMBOL(__blk_mq_end_request); void blk_mq_end_request(struct request *rq, blk_status_t error) { - if (blk_update_request(rq, error, blk_rq_bytes(rq))) - BUG(); + BUG_ON(blk_update_request(rq, error, blk_rq_bytes(rq))); __blk_mq_end_request(rq, error); } EXPORT_SYMBOL(blk_mq_end_request);
Fixes coccicheck warning: block/blk-mq.c:546:2-5: WARNING: Use BUG_ON instead of if condition followed by BUG. Fixes: 63151a449eba ("blk-mq: allow drivers to hook into I/O completion") Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Zou Wei <zou_wei@huawei.com> --- block/blk-mq.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)