iio: adc: dln2-adc: initialize local struct before using it

PATCH In review Martin Kepplinger 3 participants 5 messages Sep 10, 2017 – Sep 13, 2017 IIO

lore

Share & tools

lore

Martin Kepplinger

author

2017-09-10 12:45 UTC #0 lore raw

PATCH 1/1 iio: adc: dln2-adc: initialize local struct before using it

struct data is defined and declared locally. Initiliazation has to be done manually, so let's add that.

Signed-off-by Martin Kepplinger <[email protected]>

---

This is more of a question actually! Did you have in mind that data is not initialized here? If so, please drop this patch. This is just in case you implicitely expected data to be zero.

 drivers/iio/adc/dln2-adc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/iio/adc/dln2-adc.c +1 -1

index ab8d6aed5085..452330075b67 100644
@@ -489,7 +489,7 @@ static irqreturn_t dln2_adc_trigger_h(int irq, void *p)
 	struct {
 		__le16 values[DLN2_ADC_MAX_CHANNELS];
 		int64_t timestamp_space;
-	} data;
+	} data = { 0 };
 	struct dln2_adc_get_all_vals dev_data;
 	struct dln2_adc *dln2 = iio_priv(indio_dev);
 	const struct dln2_adc_demux_table *t;

Signature · 1 line

2.11.0

Jonathan Cameron

maintainer

2017-09-10 15:31 UTC #1 lore raw

Quoted 10 lines · On Sun, 10 Sep 2017 14:45:01 +0200 Martin Kepplinger <[email protected]> wrote:

struct data is defined and declared locally. Initiliazation has to be done manually, so let's add that.
Signed-off-by Martin Kepplinger <[email protected]>
---
This is more of a question actually! Did you have in mind that data is not initialized here? If so, please drop this patch. This is just in case you implicitely expected data to be zero.

I don't think it matters as the whole structure is filled by the later code anyway.. Am I missing a path where it isn't?

Jonathan

Quoted 17 lines

 drivers/iio/adc/dln2-adc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/iio/adc/dln2-adc.c +1 -1

index ab8d6aed5085..452330075b67 100644
@@ -489,7 +489,7 @@ static irqreturn_t dln2_adc_trigger_h(int irq, void *p)
 	struct {
 		__le16 values[DLN2_ADC_MAX_CHANNELS];
 		int64_t timestamp_space;
-	} data;
+	} data = { 0 };
 	struct dln2_adc_get_all_vals dev_data;
 	struct dln2_adc *dln2 = iio_priv(indio_dev);
 	const struct dln2_adc_demux_table *t;

kbuild test robot

build bot

2017-09-12 01:32 UTC #2 lore raw

In reply to Martin Kepplinger

reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross…

Hi Martin,

[auto build test WARNING on iio/togreg]
[also build test WARNING on next-20170911]
[cannot apply to v4.13]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Martin-Kepplinger/iio-adc-dln2-adc-initialize-local-struct-before-using-it/20170912-064250
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: tile-allyesconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 4.6.2

reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=tile

All warnings (new ones prefixed by >>):

drivers/iio/adc/dln2-adc.c: In function 'dln2_adc_trigger_h':

>> drivers/iio/adc/dln2-adc.c:492:2: warning: missing braces around initializer [-Wmissing-braces]

drivers/iio/adc/dln2-adc.c:492:2: warning: (near initialization for 'data.values') [-Wmissing-braces]

vim +492 drivers/iio/adc/dln2-adc.c

484
485 static irqreturn_t dln2_adc_trigger_h(int irq, void *p)
486 {
487 struct iio_poll_func *pf = p;
488 struct iio_dev *indio_dev = pf->indio_dev;
489 struct {
490 __le16 values[DLN2_ADC_MAX_CHANNELS];
491 int64_t timestamp_space;

 > 492		} data = { 0 };

493 struct dln2_adc_get_all_vals dev_data;
494 struct dln2_adc *dln2 = iio_priv(indio_dev);
495 const struct dln2_adc_demux_table *t;
496 int ret, i;
497
498 mutex_lock(&dln2->mutex);
499 ret = dln2_adc_read_all(dln2, &dev_data);
500 mutex_unlock(&dln2->mutex);
501 if (ret < 0)
502 goto done;
503
504 /* Demux operation */
505 for (i = 0; i < dln2->demux_count; ++i) {
506 t = &dln2->demux[i];
507 memcpy((void *)data.values + t->to,
508 (void *)dev_data.values + t->from, t->length);
509 }
510
511 /* Zero padding space between values and timestamp */
512 if (dln2->ts_pad_length)
513 memset((void *)data.values + dln2->ts_pad_offset,
514 0, dln2->ts_pad_length);
515
516 iio_push_to_buffers_with_timestamp(indio_dev, &data,
517 iio_get_time_ns(indio_dev));
518
519 done:
520 iio_trigger_notify_done(indio_dev->trig);
521 return IRQ_HANDLED;
522 }
523

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Martin Kepplinger

author

2017-09-12 07:01 UTC #3 lore raw

Am 12.09.2017 03:32 schrieb kbuild test robot:

Quoted 30 lines

Hi Martin,
[auto build test WARNING on iio/togreg]
[also build test WARNING on next-20170911]
[cannot apply to v4.13]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url:
https://github.com/0day-ci/linux/commits/Martin-Kepplinger/iio-adc-dln2-adc-initialize-local-struct-before-using-it/20170912-064250
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: tile-allyesconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 4.6.2 reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree make.cross ARCH=tile
All warnings (new ones prefixed by >>):
drivers/iio/adc/dln2-adc.c: In function 'dln2_adc_trigger_h':
Quoted 1 line
Quoted 1 line
drivers/iio/adc/dln2-adc.c:492:2: warning: missing braces around initializer [-Wmissing-braces]
drivers/iio/adc/dln2-adc.c:492:2: warning: (near initialization for
'data.values') [-Wmissing-braces]
vim +492 drivers/iio/adc/dln2-adc.c

Quoted 9 lines

   484
   485	static irqreturn_t dln2_adc_trigger_h(int irq, void *p)
   486	{
   487		struct iio_poll_func *pf = p;
   488		struct iio_dev *indio_dev = pf->indio_dev;
   489		struct {
   490			__le16 values[DLN2_ADC_MAX_CHANNELS];
   491			int64_t timestamp_space;

Quoted 1 line

492 } data = { 0 };

ok :) I guess

   } data = {{ 0 }};

would be ok for gcc in this case. How far has this patch gone? Do you want a fix on top or the patch re-done? Or do you just do it yourself?

thanks,

martin

Jonathan Cameron

maintainer

2017-09-13 16:52 UTC #4 lore raw

Quoted 44 lines · On Tue, 12 Sep 2017 09:01:27 +0200 Martin Kepplinger <[email protected]> wrote:

Am 12.09.2017 03:32 schrieb kbuild test robot:
Quoted 32 lines
Hi Martin,
[auto build test WARNING on iio/togreg]
[also build test WARNING on next-20170911]
[cannot apply to v4.13]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url:
https://github.com/0day-ci/linux/commits/Martin-Kepplinger/iio-adc-dln2-adc-initialize-local-struct-before-using-it/20170912-064250
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
togreg
config: tile-allyesconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 4.6.2 reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree make.cross ARCH=tile
All warnings (new ones prefixed by >>):
drivers/iio/adc/dln2-adc.c: In function 'dln2_adc_trigger_h':
Quoted 2 lines
Quoted 2 lines
drivers/iio/adc/dln2-adc.c:492:2: warning: missing braces around initializer [-Wmissing-braces]
drivers/iio/adc/dln2-adc.c:492:2: warning: (near initialization for
'data.values') [-Wmissing-braces]
vim +492 drivers/iio/adc/dln2-adc.c
Quoted 9 lines
   484
   485	static irqreturn_t dln2_adc_trigger_h(int irq, void *p)
   486	{
   487		struct iio_poll_func *pf = p;
   488		struct iio_dev *indio_dev = pf->indio_dev;
   489		struct {
   490			__le16 values[DLN2_ADC_MAX_CHANNELS];
   491			int64_t timestamp_space;  
Quoted 1 line
492 } data = { 0 };

Quoted 8 lines

ok :) I guess
    } data = {{ 0 }};
would be ok for gcc in this case. How far has this patch gone? Do you want a fix
on top or the patch re-done? Or do you just do it yourself?

I'm unconvinced the patch is necessary as I think all elements of that structure are initialized anyway before they are used.

I might be missing a code path where that isn't true though so feel free to point one out!

Jonathan

Quoted 3 lines

thanks,
martin