| Message ID | 20210921133749.15461-1-tim.gardner@canonical.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v3,next] counter: Add default statement to switch() in quad8_function_read() | expand |
On Tue, Sep 21, 2021 at 07:37:49AM -0600, Tim Gardner wrote: > Coverity complains of a possible use of an uninitialized variable in > quad8_action_read(). > > CID 119643 (#1 of 1): Uninitialized scalar variable (UNINIT) > 4. uninit_use: Using uninitialized value function. > 346 switch (function) { > > The call to quad8_function_read() could theoretically return without assigning > a value to '*function', thus causing the use of an ininitialized variable > 'function' in quad8_action_read(). > > Fix this by adding a default statement to the switch in quad8_function_read() > and setting a return error code. > > Cc: William Breathitt Gray <vilhelm.gray@gmail.com> > Cc: Syed Nayyar Waris <syednwaris@gmail.com> > Cc: linux-iio@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> Thank you for noticing the mutex. Although this case is simple, I'd still prefer for this function to return early when an error is found rather than hold a return value until the end. Please adjust the default case to unlock the mutex directly and return immediately with -EINVAL. With that change feel free to add my Ack-by line: Acked-by: William Breathitt Gray <vilhelm.gray@gmail.com> > --- > v2 - Add the correct Cc's > v3 - Add comment to the default switch statement. Also noticed v2 would have > returned with a lock held. Fix that by returning a variable return code. > --- > drivers/counter/104-quad-8.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c > index c587f295d720..7faca6b760e7 100644 > --- a/drivers/counter/104-quad-8.c > +++ b/drivers/counter/104-quad-8.c > @@ -201,6 +201,7 @@ static int quad8_function_read(struct counter_device *counter, > { > struct quad8 *const priv = counter->priv; > const int id = count->id; > + int ret = 0; > > mutex_lock(&priv->lock); > > @@ -215,13 +216,16 @@ static int quad8_function_read(struct counter_device *counter, > case 2: > *function = COUNTER_FUNCTION_QUADRATURE_X4; > break; > + default: > + /* should never reach this path */ > + ret = -EINVAL; > } > else > *function = COUNTER_FUNCTION_PULSE_DIRECTION; > > mutex_unlock(&priv->lock); > > - return 0; > + return ret; > } > > static int quad8_function_write(struct counter_device *counter, > -- > 2.33.0 >
diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c index c587f295d720..7faca6b760e7 100644 --- a/drivers/counter/104-quad-8.c +++ b/drivers/counter/104-quad-8.c @@ -201,6 +201,7 @@ static int quad8_function_read(struct counter_device *counter, { struct quad8 *const priv = counter->priv; const int id = count->id; + int ret = 0; mutex_lock(&priv->lock); @@ -215,13 +216,16 @@ static int quad8_function_read(struct counter_device *counter, case 2: *function = COUNTER_FUNCTION_QUADRATURE_X4; break; + default: + /* should never reach this path */ + ret = -EINVAL; } else *function = COUNTER_FUNCTION_PULSE_DIRECTION; mutex_unlock(&priv->lock); - return 0; + return ret; } static int quad8_function_write(struct counter_device *counter,
Coverity complains of a possible use of an uninitialized variable in quad8_action_read(). CID 119643 (#1 of 1): Uninitialized scalar variable (UNINIT) 4. uninit_use: Using uninitialized value function. 346 switch (function) { The call to quad8_function_read() could theoretically return without assigning a value to '*function', thus causing the use of an ininitialized variable 'function' in quad8_action_read(). Fix this by adding a default statement to the switch in quad8_function_read() and setting a return error code. Cc: William Breathitt Gray <vilhelm.gray@gmail.com> Cc: Syed Nayyar Waris <syednwaris@gmail.com> Cc: linux-iio@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Tim Gardner <tim.gardner@canonical.com> --- v2 - Add the correct Cc's v3 - Add comment to the default switch statement. Also noticed v2 would have returned with a lock held. Fix that by returning a variable return code. --- drivers/counter/104-quad-8.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)