Re: [PATCH] staging: comedi: tio: fix multiple missing break in switch bugs

[Gustavo A. R. Silva]

On 10/12/18 11:04 AM, Ian Abbott wrote:
> On 11/10/2018 20:05, Gustavo A. R. Silva wrote:
>> Currently, there are multiple missing break statements in two switch code
>> blocks. This makes the execution path to fall all the way down through
>> to the default cases, which makes the function ni_tio_set_gate_src() to
>> always return -EINVAL.
>>
>> Fix this by adding the missing break statements.
>>
>> Also, notice that due to the absence of the break statements,
>> the following pieces of code are unreachable:
>>
>> 1078ÂÂÂ if (ret)
>> 1079ÂÂÂÂÂÂÂÂÂÂÂ return ret;
>> 1080 /* 3. reenable & set mode to starts things back up */
>> 1081ÂÂÂ ni_tio_set_gate_mode(counter, src);
>>
>> 1098ÂÂÂ if (ret)
>> 1099ÂÂÂÂÂÂÂÂÂÂÂ return ret;
>> 1100 /* 3. reenable & set mode to starts things back up */
>> 1101ÂÂÂ ni_tio_set_gate2_mode(counter, src);
>>
>> So, by adding the missing breaks, this patch also fixes the problem
>> above.
>>
>> Addresses-Coverity-ID: 1474165 ("Missing break in switch")
>> Addresses-Coverity-ID: 1474162 ("Structurally dead code")
>> Fixes: 347e244884c3 ("staging: comedi: tio: implement global tio/ctr routing")
>> Signed-off-by: Gustavo A. R. Silva <gustavo@xxxxxxxxxxxxxx>
>> ---
>> Â drivers/staging/comedi/drivers/ni_tio.c | 4 ++++
>> Â 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/staging/comedi/drivers/ni_tio.c b/drivers/staging/comedi/drivers/ni_tio.c
>> index 838614e..0eb388c 100644
>> --- a/drivers/staging/comedi/drivers/ni_tio.c
>> +++ b/drivers/staging/comedi/drivers/ni_tio.c
>> @@ -1070,8 +1070,10 @@ int ni_tio_set_gate_src(struct ni_gpct *counter,
>> ÂÂÂÂÂÂÂÂÂ case ni_gpct_variant_e_series:
>> ÂÂÂÂÂÂÂÂÂ case ni_gpct_variant_m_series:
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ ret = ni_m_set_gate(counter, chan);
>> +ÂÂÂÂÂÂÂÂÂÂÂ break;
>> ÂÂÂÂÂÂÂÂÂ case ni_gpct_variant_660x:
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ ret = ni_660x_set_gate(counter, chan);
>> +ÂÂÂÂÂÂÂÂÂÂÂ break;
>> ÂÂÂÂÂÂÂÂÂ default:
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ return -EINVAL;
>> ÂÂÂÂÂÂÂÂÂ }
>> @@ -1090,8 +1092,10 @@ int ni_tio_set_gate_src(struct ni_gpct *counter,
>> ÂÂÂÂÂÂÂÂÂ switch (counter_dev->variant) {
>> ÂÂÂÂÂÂÂÂÂ case ni_gpct_variant_m_series:
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ ret = ni_m_set_gate2(counter, chan);
>> +ÂÂÂÂÂÂÂÂÂÂÂ break;
>> ÂÂÂÂÂÂÂÂÂ case ni_gpct_variant_660x:
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ ret = ni_660x_set_gate2(counter, chan);
>> +ÂÂÂÂÂÂÂÂÂÂÂ break;
>> ÂÂÂÂÂÂÂÂÂ default:
>> ÂÂÂÂÂÂÂÂÂÂÂÂÂ return -EINVAL;
>> ÂÂÂÂÂÂÂÂÂ }
>>
> 
> Thanks for catching that bug!
> 

Glad to help. :)

> Reviewed-by: Ian Abbott <abbotti@xxxxxxxxx>
> 

Thanks
--
Gustavo