| Message ID | 20190322140937.341976-1-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | iio: ssp_sensors: avoid uninitialized variable usage | expand |
On Fri, Mar 22, 2019 at 03:09:22PM +0100, Arnd Bergmann wrote: > clang points out that 'calculated_time' is only sometimes > initialized here, which leads to incorrect data being > passed into another function: > > drivers/iio/common/ssp_sensors/ssp_iio.c:95:6: error: variable 'calculated_time' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] > if (indio_dev->scan_timestamp) { > ^~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/iio/common/ssp_sensors/ssp_iio.c:102:9: note: uninitialized use occurs here > calculated_time); > ^~~~~~~~~~~~~~~ > drivers/iio/common/ssp_sensors/ssp_iio.c:95:2: note: remove the 'if' if its condition is always true > if (indio_dev->scan_timestamp) { > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/iio/common/ssp_sensors/ssp_iio.c:84:25: note: initialize the variable 'calculated_time' to silence this warning > int64_t calculated_time; > ^ > The data is subsequently ignored by iio_push_to_buffers_with_timestamp(), > but the warning still feels legitimate and to work around it, we can > initialize the time in the other case. > > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=18501 > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/iio/common/ssp_sensors/ssp_iio.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/iio/common/ssp_sensors/ssp_iio.c b/drivers/iio/common/ssp_sensors/ssp_iio.c > index 645f2e3975db..81e8f4844c90 100644 > --- a/drivers/iio/common/ssp_sensors/ssp_iio.c > +++ b/drivers/iio/common/ssp_sensors/ssp_iio.c > @@ -96,6 +96,8 @@ int ssp_common_process_data(struct iio_dev *indio_dev, void *buf, > memcpy(&time, &((char *)buf)[len], SSP_TIME_SIZE); > calculated_time = > timestamp + (int64_t)le32_to_cpu(time) * 1000000; > + } else { > + calculated_time = 0; > } > > return iio_push_to_buffers_with_timestamp(indio_dev, spd->buffer, > -- > 2.20.0 > I sent a similar change, which is sitting in Jonathan's testing branch: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?id=0643039b4fee4aa54a233ead15dc0b2286f059d7 You made a good point previously that initializing the variable at the beginning of a function may not always be the best choice. I don't have a personal preference for which patch stays around so: Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> Just in case. Nathan
On Fri, 22 Mar 2019 08:01:19 -0700 Nathan Chancellor <natechancellor@gmail.com> wrote: > On Fri, Mar 22, 2019 at 03:09:22PM +0100, Arnd Bergmann wrote: > > clang points out that 'calculated_time' is only sometimes > > initialized here, which leads to incorrect data being > > passed into another function: > > > > drivers/iio/common/ssp_sensors/ssp_iio.c:95:6: error: variable 'calculated_time' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] > > if (indio_dev->scan_timestamp) { > > ^~~~~~~~~~~~~~~~~~~~~~~~~ > > drivers/iio/common/ssp_sensors/ssp_iio.c:102:9: note: uninitialized use occurs here > > calculated_time); > > ^~~~~~~~~~~~~~~ > > drivers/iio/common/ssp_sensors/ssp_iio.c:95:2: note: remove the 'if' if its condition is always true > > if (indio_dev->scan_timestamp) { > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > drivers/iio/common/ssp_sensors/ssp_iio.c:84:25: note: initialize the variable 'calculated_time' to silence this warning > > int64_t calculated_time; > > ^ > > The data is subsequently ignored by iio_push_to_buffers_with_timestamp(), > > but the warning still feels legitimate and to work around it, we can > > initialize the time in the other case. > > > > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=18501 > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > --- > > drivers/iio/common/ssp_sensors/ssp_iio.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/iio/common/ssp_sensors/ssp_iio.c b/drivers/iio/common/ssp_sensors/ssp_iio.c > > index 645f2e3975db..81e8f4844c90 100644 > > --- a/drivers/iio/common/ssp_sensors/ssp_iio.c > > +++ b/drivers/iio/common/ssp_sensors/ssp_iio.c > > @@ -96,6 +96,8 @@ int ssp_common_process_data(struct iio_dev *indio_dev, void *buf, > > memcpy(&time, &((char *)buf)[len], SSP_TIME_SIZE); > > calculated_time = > > timestamp + (int64_t)le32_to_cpu(time) * 1000000; > > + } else { > > + calculated_time = 0; > > } > > > > return iio_push_to_buffers_with_timestamp(indio_dev, spd->buffer, > > -- > > 2.20.0 > > > > I sent a similar change, which is sitting in Jonathan's testing branch: > > https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?id=0643039b4fee4aa54a233ead15dc0b2286f059d7 > > You made a good point previously that initializing the variable at the > beginning of a function may not always be the best choice. I don't have > a personal preference for which patch stays around so: > > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > > Just in case. > > Nathan I'll stick to the original, mostly to avoid unnecessary churn. Apologies for the delay in pull requests making these more generally visible. Always takes me a while to get one out after RC1. Would imagine these will go to Greg and hence linux-next sometime in the next week. Jonathan
diff --git a/drivers/iio/common/ssp_sensors/ssp_iio.c b/drivers/iio/common/ssp_sensors/ssp_iio.c index 645f2e3975db..81e8f4844c90 100644 --- a/drivers/iio/common/ssp_sensors/ssp_iio.c +++ b/drivers/iio/common/ssp_sensors/ssp_iio.c @@ -96,6 +96,8 @@ int ssp_common_process_data(struct iio_dev *indio_dev, void *buf, memcpy(&time, &((char *)buf)[len], SSP_TIME_SIZE); calculated_time = timestamp + (int64_t)le32_to_cpu(time) * 1000000; + } else { + calculated_time = 0; } return iio_push_to_buffers_with_timestamp(indio_dev, spd->buffer,
clang points out that 'calculated_time' is only sometimes initialized here, which leads to incorrect data being passed into another function: drivers/iio/common/ssp_sensors/ssp_iio.c:95:6: error: variable 'calculated_time' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] if (indio_dev->scan_timestamp) { ^~~~~~~~~~~~~~~~~~~~~~~~~ drivers/iio/common/ssp_sensors/ssp_iio.c:102:9: note: uninitialized use occurs here calculated_time); ^~~~~~~~~~~~~~~ drivers/iio/common/ssp_sensors/ssp_iio.c:95:2: note: remove the 'if' if its condition is always true if (indio_dev->scan_timestamp) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/iio/common/ssp_sensors/ssp_iio.c:84:25: note: initialize the variable 'calculated_time' to silence this warning int64_t calculated_time; ^ The data is subsequently ignored by iio_push_to_buffers_with_timestamp(), but the warning still feels legitimate and to work around it, we can initialize the time in the other case. Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=18501 Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/iio/common/ssp_sensors/ssp_iio.c | 2 ++ 1 file changed, 2 insertions(+) -- 2.20.0