| Message ID | 20181003121211.GA28108@embeddedor.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spi: pxa2xx: Mark expected switch fall-through | expand |
On Wed, Oct 03, 2018 at 02:12:11PM +0200, Gustavo A. R. Silva wrote: > switch (drv_data->n_bytes) { > case 4: > bytes_left >>= 1; > + /* Fall through */ > case 2: > bytes_left >>= 1; > } I think this code is just being too cute and it'd be better to just rewrite it to directly do the expected number of shifts directly in each case and have break statements; your fix is good but still not ideal for readability I think.
On 10/3/18 5:22 PM, Mark Brown wrote: > On Wed, Oct 03, 2018 at 02:12:11PM +0200, Gustavo A. R. Silva wrote: > >> switch (drv_data->n_bytes) { >> case 4: >> bytes_left >>= 1; >> + /* Fall through */ >> case 2: >> bytes_left >>= 1; >> } > > I think this code is just being too cute and it'd be better to just > rewrite it to directly do the expected number of shifts directly in each > case and have break statements; your fix is good but still not ideal for > readability I think. > Okay. I agree. I'll rewrite and send v2. Thanks -- Gustavo
On 10/3/18 5:27 PM, Gustavo A. R. Silva wrote: > > > On 10/3/18 5:22 PM, Mark Brown wrote: >> On Wed, Oct 03, 2018 at 02:12:11PM +0200, Gustavo A. R. Silva wrote: >> >>> switch (drv_data->n_bytes) { >>> case 4: >>> bytes_left >>= 1; >>> + /* Fall through */ >>> case 2: >>> bytes_left >>= 1; >>> } >> >> I think this code is just being too cute and it'd be better to just >> rewrite it to directly do the expected number of shifts directly in each >> case and have break statements; your fix is good but still not ideal for >> readability I think. >> > > Okay. I agree. I'll rewrite and send v2. > I'll actually send a completely new patch for this. Thanks -- Gustavo
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c index fc9aac2..728b5f3 100644 --- a/drivers/spi/spi-pxa2xx.c +++ b/drivers/spi/spi-pxa2xx.c @@ -666,6 +666,7 @@ static irqreturn_t interrupt_transfer(struct driver_data *drv_data) switch (drv_data->n_bytes) { case 4: bytes_left >>= 1; + /* Fall through */ case 2: bytes_left >>= 1; }
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1056539 ("Missing break in switch") Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> --- drivers/spi/spi-pxa2xx.c | 1 + 1 file changed, 1 insertion(+)