| Message ID | 20210701010433.GA57746@embeddedor (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4e1c8c17ff129ab14a38c461dd9bb8f7ff8a36a0 |
| Headers | show |
| Series | scsi: aic94xx: Fix fall-through warning for Clang | expand |
On Wed, 2021-06-30 at 20:04 -0500, Gustavo A. R. Silva wrote: > In preparation to enable -Wimplicit-fallthrough for Clang, fix a > warning by explicitly adding a fallthrough; statement. > > Notice that this seems to be a Duff device for performance[1]. So, > although the code looks a bit _funny_, I didn't want to refactor > or modify it beyond merely adding a fallthrough marking, which > might be the least disruptive way to fix this issue. If you read the surrounding calling code, it appears it's not performant. * @start has to be the _base_ element start, since the * linked list entries's offset is from this pointer. * Some linked list entries use only the first id, in which case * you can pass 0xFF for the second. > diff --git a/drivers/scsi/aic94xx/aic94xx_sds.c b/drivers/scsi/aic94xx/aic94xx_sds.c [] > @@ -718,10 +718,12 @@ static void *asd_find_ll_by_id(void * const start, const u8 id0, const u8 id1) > do { > switch (id1) { > default: > - if (el->id1 == id1) > + if (el->id1 == id1) { > + fallthrough; > case 0xFF: > if (el->id0 == id0) > return el; > + } > } > el = start + le16_to_cpu(el->next); > } while (el != start); And this is still horrible code to read. I think the below is rather more sensible and it doesn't change the defconfig with aic94xx object size. $ size drivers/scsi/aic94xx/aic94xx_sds.o* text data bss dec hex filename 10010 192 0 10202 27da drivers/scsi/aic94xx/aic94xx_sds.o.new 10010 192 0 10202 27da drivers/scsi/aic94xx/aic94xx_sds.o.old --- drivers/scsi/aic94xx/aic94xx_sds.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/aic94xx/aic94xx_sds.c b/drivers/scsi/aic94xx/aic94xx_sds.c index 297a66770260c..069278f6d6cea 100644 --- a/drivers/scsi/aic94xx/aic94xx_sds.c +++ b/drivers/scsi/aic94xx/aic94xx_sds.c @@ -717,11 +717,14 @@ static void *asd_find_ll_by_id(void * const start, const u8 id0, const u8 id1) do { switch (id1) { - default: - if (el->id1 == id1) case 0xFF: - if (el->id0 == id0) - return el; + if (el->id0 == id0) + return el; + break; + default: + if (el->id0 == id0 && el->id1 == id1) + return el; + break; } el = start + le16_to_cpu(el->next); } while (el != start);
diff --git a/drivers/scsi/aic94xx/aic94xx_sds.c b/drivers/scsi/aic94xx/aic94xx_sds.c index 297a66770260..46815e65f7a4 100644 --- a/drivers/scsi/aic94xx/aic94xx_sds.c +++ b/drivers/scsi/aic94xx/aic94xx_sds.c @@ -718,10 +718,12 @@ static void *asd_find_ll_by_id(void * const start, const u8 id0, const u8 id1) do { switch (id1) { default: - if (el->id1 == id1) + if (el->id1 == id1) { + fallthrough; case 0xFF: if (el->id0 == id0) return el; + } } el = start + le16_to_cpu(el->next); } while (el != start);
In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning by explicitly adding a fallthrough; statement. Notice that this seems to be a Duff device for performance[1]. So, although the code looks a bit _funny_, I didn't want to refactor or modify it beyond merely adding a fallthrough marking, which might be the least disruptive way to fix this issue. [1] https://www.drdobbs.com/a-reusable-duff-device/184406208 Link: https://github.com/KSPP/linux/issues/115 Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- JFYI: We had thousands of these sorts of warnings and now we are down to just 7 in linux-next(20210630). This is one of those last remaining warnings. :) drivers/scsi/aic94xx/aic94xx_sds.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)