* [PATCH] rtc: remove VLA usage @ 2018-03-07 20:11 Gustavo A. R. Silva 2018-03-07 21:25 ` Alexandre Belloni 0 siblings, 1 reply; 8+ messages in thread From: Gustavo A. R. Silva @ 2018-03-07 20:11 UTC (permalink / raw) To: Alessandro Zummo, Alexandre Belloni Cc: linux-rtc, linux-kernel, Gustavo A. R. Silva In preparation to enabling -Wvla, remove VLA and replace it with a fixed-length array instead. Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> --- drivers/rtc/rtc-bq32k.c | 2 +- drivers/rtc/rtc-mcp795.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c index e8698e9..e4b234a 100644 --- a/drivers/rtc/rtc-bq32k.c +++ b/drivers/rtc/rtc-bq32k.c @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) { struct i2c_client *client = to_i2c_client(dev); - uint8_t buffer[len + 1]; + uint8_t buffer[256]; buffer[0] = off; memcpy(&buffer[1], data, len); diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c index 79e24ea..00e11c1 100644 --- a/drivers/rtc/rtc-mcp795.c +++ b/drivers/rtc/rtc-mcp795.c @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) { struct spi_device *spi = to_spi_device(dev); int ret; - u8 tx[2 + count]; + u8 tx[257]; tx[0] = MCP795_WRITE; tx[1] = addr; -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] rtc: remove VLA usage 2018-03-07 20:11 [PATCH] rtc: remove VLA usage Gustavo A. R. Silva @ 2018-03-07 21:25 ` Alexandre Belloni 2018-03-07 22:39 ` Gustavo A. R. Silva 2018-03-07 22:40 ` Gustavo A. R. Silva 0 siblings, 2 replies; 8+ messages in thread From: Alexandre Belloni @ 2018-03-07 21:25 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Alessandro Zummo, linux-rtc, linux-kernel, Gustavo A. R. Silva On 07/03/2018 at 14:11:33 -0600, Gustavo A. R. Silva wrote: > In preparation to enabling -Wvla, remove VLA and replace it > with a fixed-length array instead. > You should probably explain what VLA is and why this is important to do. > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > --- > drivers/rtc/rtc-bq32k.c | 2 +- > drivers/rtc/rtc-mcp795.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c > index e8698e9..e4b234a 100644 > --- a/drivers/rtc/rtc-bq32k.c > +++ b/drivers/rtc/rtc-bq32k.c > @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) > static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) > { > struct i2c_client *client = to_i2c_client(dev); > - uint8_t buffer[len + 1]; > + uint8_t buffer[256]; > You chose to change len to 255, probably because this is a uint8_t but this is way too much for this rtc, it only has 10 consecutive registers. > buffer[0] = off; > memcpy(&buffer[1], data, len); > diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c > index 79e24ea..00e11c1 100644 > --- a/drivers/rtc/rtc-mcp795.c > +++ b/drivers/rtc/rtc-mcp795.c > @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) > { > struct spi_device *spi = to_spi_device(dev); > int ret; > - u8 tx[2 + count]; > + u8 tx[257]; > > tx[0] = MCP795_WRITE; > tx[1] = addr; > -- > 2.7.4 > -- Alexandre Belloni, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rtc: remove VLA usage 2018-03-07 21:25 ` Alexandre Belloni @ 2018-03-07 22:39 ` Gustavo A. R. Silva 2018-03-07 23:01 ` Alexandre Belloni 2018-03-07 22:40 ` Gustavo A. R. Silva 1 sibling, 1 reply; 8+ messages in thread From: Gustavo A. R. Silva @ 2018-03-07 22:39 UTC (permalink / raw) To: Alexandre Belloni, Gustavo A. R. Silva Cc: Alessandro Zummo, linux-rtc, linux-kernel Hi Alexandre, On 03/07/2018 03:25 PM, Alexandre Belloni wrote: > On 07/03/2018 at 14:11:33 -0600, Gustavo A. R. Silva wrote: >> In preparation to enabling -Wvla, remove VLA and replace it >> with a fixed-length array instead. >> > You should probably explain what VLA is and why this is important to do. Sure. I can elaborate a little bit more. >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >> --- >> drivers/rtc/rtc-bq32k.c | 2 +- >> drivers/rtc/rtc-mcp795.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c >> index e8698e9..e4b234a 100644 >> --- a/drivers/rtc/rtc-bq32k.c >> +++ b/drivers/rtc/rtc-bq32k.c >> @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) >> static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) >> { >> struct i2c_client *client = to_i2c_client(dev); >> - uint8_t buffer[len + 1]; >> + uint8_t buffer[256]; >> > You chose to change len to 255, probably because this is a uint8_t but Correct. > this is way too much for this rtc, it only has 10 consecutive registers. In that case probably the best solution is to add the following line to the module: #define MAX_LEN 10 and update the rest of the code as follows: uint8_t buffer[MAX_LEN + 1]; >> buffer[0] = off; >> memcpy(&buffer[1], data, len); >> diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c >> index 79e24ea..00e11c1 100644 >> --- a/drivers/rtc/rtc-mcp795.c >> +++ b/drivers/rtc/rtc-mcp795.c >> @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) >> { >> struct spi_device *spi = to_spi_device(dev); >> int ret; >> - u8 tx[2 + count]; >> + u8 tx[257]; For this particular case it seems to me that the following works just fine: #define MAX_COUNT 7 u8 tx[MAX_COUNT + 2]; What do you think? Thanks for your feedback. -- Gustavo >> >> tx[0] = MCP795_WRITE; >> tx[1] = addr; >> -- >> 2.7.4 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rtc: remove VLA usage 2018-03-07 22:39 ` Gustavo A. R. Silva @ 2018-03-07 23:01 ` Alexandre Belloni 2018-03-07 23:09 ` Gustavo A. R. Silva 0 siblings, 1 reply; 8+ messages in thread From: Alexandre Belloni @ 2018-03-07 23:01 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Gustavo A. R. Silva, Alessandro Zummo, linux-rtc, linux-kernel On 07/03/2018 at 16:39:51 -0600, Gustavo A. R. Silva wrote: > Hi Alexandre, > > On 03/07/2018 03:25 PM, Alexandre Belloni wrote: > > On 07/03/2018 at 14:11:33 -0600, Gustavo A. R. Silva wrote: > > > In preparation to enabling -Wvla, remove VLA and replace it > > > with a fixed-length array instead. > > > > > You should probably explain what VLA is and why this is important to do. > > Sure. I can elaborate a little bit more. > > > > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > > > --- > > > drivers/rtc/rtc-bq32k.c | 2 +- > > > drivers/rtc/rtc-mcp795.c | 2 +- > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c > > > index e8698e9..e4b234a 100644 > > > --- a/drivers/rtc/rtc-bq32k.c > > > +++ b/drivers/rtc/rtc-bq32k.c > > > @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) > > > static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) > > > { > > > struct i2c_client *client = to_i2c_client(dev); > > > - uint8_t buffer[len + 1]; > > > + uint8_t buffer[256]; > > You chose to change len to 255, probably because this is a uint8_t but > > Correct. > > > this is way too much for this rtc, it only has 10 consecutive registers. > > In that case probably the best solution is to add the following line to the > module: > > #define MAX_LEN 10 > > and update the rest of the code as follows: > > uint8_t buffer[MAX_LEN + 1]; > Seems better, yes. > > > > buffer[0] = off; > > > memcpy(&buffer[1], data, len); > > > diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c > > > index 79e24ea..00e11c1 100644 > > > --- a/drivers/rtc/rtc-mcp795.c > > > +++ b/drivers/rtc/rtc-mcp795.c > > > @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) > > > { > > > struct spi_device *spi = to_spi_device(dev); > > > int ret; > > > - u8 tx[2 + count]; > > > + u8 tx[257]; > > For this particular case it seems to me that the following works just fine: > > #define MAX_COUNT 7 > > u8 tx[MAX_COUNT + 2]; > > What do you think? > Nope, that RTC has a section of 255 bytes that could be read at once so 257 is the correct value. -- Alexandre Belloni, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rtc: remove VLA usage 2018-03-07 23:01 ` Alexandre Belloni @ 2018-03-07 23:09 ` Gustavo A. R. Silva 2018-03-07 23:12 ` Alexandre Belloni 0 siblings, 1 reply; 8+ messages in thread From: Gustavo A. R. Silva @ 2018-03-07 23:09 UTC (permalink / raw) To: Alexandre Belloni Cc: Gustavo A. R. Silva, Alessandro Zummo, linux-rtc, linux-kernel On 03/07/2018 05:01 PM, Alexandre Belloni wrote: > On 07/03/2018 at 16:39:51 -0600, Gustavo A. R. Silva wrote: >> Hi Alexandre, >> >> On 03/07/2018 03:25 PM, Alexandre Belloni wrote: >>> On 07/03/2018 at 14:11:33 -0600, Gustavo A. R. Silva wrote: >>>> In preparation to enabling -Wvla, remove VLA and replace it >>>> with a fixed-length array instead. >>>> >>> You should probably explain what VLA is and why this is important to do. >> Sure. I can elaborate a little bit more. >> >>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >>>> --- >>>> drivers/rtc/rtc-bq32k.c | 2 +- >>>> drivers/rtc/rtc-mcp795.c | 2 +- >>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c >>>> index e8698e9..e4b234a 100644 >>>> --- a/drivers/rtc/rtc-bq32k.c >>>> +++ b/drivers/rtc/rtc-bq32k.c >>>> @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) >>>> static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) >>>> { >>>> struct i2c_client *client = to_i2c_client(dev); >>>> - uint8_t buffer[len + 1]; >>>> + uint8_t buffer[256]; >>> You chose to change len to 255, probably because this is a uint8_t but >> Correct. >> >>> this is way too much for this rtc, it only has 10 consecutive registers. >> In that case probably the best solution is to add the following line to the >> module: >> >> #define MAX_LEN 10 >> >> and update the rest of the code as follows: >> >> uint8_t buffer[MAX_LEN + 1]; >> > Seems better, yes. > >>>> buffer[0] = off; >>>> memcpy(&buffer[1], data, len); >>>> diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c >>>> index 79e24ea..00e11c1 100644 >>>> --- a/drivers/rtc/rtc-mcp795.c >>>> +++ b/drivers/rtc/rtc-mcp795.c >>>> @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) >>>> { >>>> struct spi_device *spi = to_spi_device(dev); >>>> int ret; >>>> - u8 tx[2 + count]; >>>> + u8 tx[257]; >> For this particular case it seems to me that the following works just fine: >> >> #define MAX_COUNT 7 >> >> u8 tx[MAX_COUNT + 2]; >> >> What do you think? >> > Nope, that RTC has a section of 255 bytes that could be read at once so > 257 is the correct value. > I see... I was looking into this piece of code drivers/rtc/rtc-mcp795.c:302: tmp[0] = (tmp[0] & 0x80) | bin2bcd(alm->time.tm_sec); tmp[1] = (tmp[1] & 0x80) | bin2bcd(alm->time.tm_min); tmp[2] = (tmp[2] & 0xE0) | bin2bcd(alm->time.tm_hour); tmp[3] = (tmp[3] & 0x80) | bin2bcd(alm->time.tm_wday + 1); /* set alarm match: seconds, minutes, hour, day, date and month */ tmp[3] |= (MCP795_ALM0C2_BIT | MCP795_ALM0C1_BIT | MCP795_ALM0C0_BIT); tmp[4] = (tmp[4] & 0xC0) | bin2bcd(alm->time.tm_mday); tmp[5] = (tmp[5] & 0xE0) | bin2bcd(alm->time.tm_mon + 1); ret = mcp795_rtcc_write(dev, MCP795_REG_ALM0_SECONDS, tmp, sizeof(tmp)); OK. I'll work on v2 of this patch. I'll send it shortly. Thanks -- Gustavo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rtc: remove VLA usage 2018-03-07 23:09 ` Gustavo A. R. Silva @ 2018-03-07 23:12 ` Alexandre Belloni 2018-03-07 23:14 ` Gustavo A. R. Silva 0 siblings, 1 reply; 8+ messages in thread From: Alexandre Belloni @ 2018-03-07 23:12 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Gustavo A. R. Silva, Alessandro Zummo, linux-rtc, linux-kernel On 07/03/2018 at 17:09:22 -0600, Gustavo A. R. Silva wrote: > > > On 03/07/2018 05:01 PM, Alexandre Belloni wrote: > > On 07/03/2018 at 16:39:51 -0600, Gustavo A. R. Silva wrote: > > > Hi Alexandre, > > > > > > On 03/07/2018 03:25 PM, Alexandre Belloni wrote: > > > > On 07/03/2018 at 14:11:33 -0600, Gustavo A. R. Silva wrote: > > > > > In preparation to enabling -Wvla, remove VLA and replace it > > > > > with a fixed-length array instead. > > > > > > > > > You should probably explain what VLA is and why this is important to do. > > > Sure. I can elaborate a little bit more. > > > > > > > > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > > > > > --- > > > > > drivers/rtc/rtc-bq32k.c | 2 +- > > > > > drivers/rtc/rtc-mcp795.c | 2 +- > > > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c > > > > > index e8698e9..e4b234a 100644 > > > > > --- a/drivers/rtc/rtc-bq32k.c > > > > > +++ b/drivers/rtc/rtc-bq32k.c > > > > > @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) > > > > > static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) > > > > > { > > > > > struct i2c_client *client = to_i2c_client(dev); > > > > > - uint8_t buffer[len + 1]; > > > > > + uint8_t buffer[256]; > > > > You chose to change len to 255, probably because this is a uint8_t but > > > Correct. > > > > > > > this is way too much for this rtc, it only has 10 consecutive registers. > > > In that case probably the best solution is to add the following line to the > > > module: > > > > > > #define MAX_LEN 10 > > > > > > and update the rest of the code as follows: > > > > > > uint8_t buffer[MAX_LEN + 1]; > > > > > Seems better, yes. > > > > > > > buffer[0] = off; > > > > > memcpy(&buffer[1], data, len); > > > > > diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c > > > > > index 79e24ea..00e11c1 100644 > > > > > --- a/drivers/rtc/rtc-mcp795.c > > > > > +++ b/drivers/rtc/rtc-mcp795.c > > > > > @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) > > > > > { > > > > > struct spi_device *spi = to_spi_device(dev); > > > > > int ret; > > > > > - u8 tx[2 + count]; > > > > > + u8 tx[257]; > > > For this particular case it seems to me that the following works just fine: > > > > > > #define MAX_COUNT 7 > > > > > > u8 tx[MAX_COUNT + 2]; > > > > > > What do you think? > > > > > Nope, that RTC has a section of 255 bytes that could be read at once so > > 257 is the correct value. > > > > I see... > > I was looking into this piece of code drivers/rtc/rtc-mcp795.c:302: > > tmp[0] = (tmp[0] & 0x80) | bin2bcd(alm->time.tm_sec); > tmp[1] = (tmp[1] & 0x80) | bin2bcd(alm->time.tm_min); > tmp[2] = (tmp[2] & 0xE0) | bin2bcd(alm->time.tm_hour); > tmp[3] = (tmp[3] & 0x80) | bin2bcd(alm->time.tm_wday + 1); > /* set alarm match: seconds, minutes, hour, day, date and month */ > tmp[3] |= (MCP795_ALM0C2_BIT | MCP795_ALM0C1_BIT | > MCP795_ALM0C0_BIT); > tmp[4] = (tmp[4] & 0xC0) | bin2bcd(alm->time.tm_mday); > tmp[5] = (tmp[5] & 0xE0) | bin2bcd(alm->time.tm_mon + 1); > > ret = mcp795_rtcc_write(dev, MCP795_REG_ALM0_SECONDS, tmp, > sizeof(tmp)); > Yeah, the 128 or 255 bytes NVRAM is not yet supported but this is on my todo list -- Alexandre Belloni, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rtc: remove VLA usage 2018-03-07 23:12 ` Alexandre Belloni @ 2018-03-07 23:14 ` Gustavo A. R. Silva 0 siblings, 0 replies; 8+ messages in thread From: Gustavo A. R. Silva @ 2018-03-07 23:14 UTC (permalink / raw) To: Alexandre Belloni Cc: Gustavo A. R. Silva, Alessandro Zummo, linux-rtc, linux-kernel On 03/07/2018 05:12 PM, Alexandre Belloni wrote: > On 07/03/2018 at 17:09:22 -0600, Gustavo A. R. Silva wrote: >> >> On 03/07/2018 05:01 PM, Alexandre Belloni wrote: >>> On 07/03/2018 at 16:39:51 -0600, Gustavo A. R. Silva wrote: >>>> Hi Alexandre, >>>> >>>> On 03/07/2018 03:25 PM, Alexandre Belloni wrote: >>>>> On 07/03/2018 at 14:11:33 -0600, Gustavo A. R. Silva wrote: >>>>>> In preparation to enabling -Wvla, remove VLA and replace it >>>>>> with a fixed-length array instead. >>>>>> >>>>> You should probably explain what VLA is and why this is important to do. >>>> Sure. I can elaborate a little bit more. >>>> >>>>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >>>>>> --- >>>>>> drivers/rtc/rtc-bq32k.c | 2 +- >>>>>> drivers/rtc/rtc-mcp795.c | 2 +- >>>>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c >>>>>> index e8698e9..e4b234a 100644 >>>>>> --- a/drivers/rtc/rtc-bq32k.c >>>>>> +++ b/drivers/rtc/rtc-bq32k.c >>>>>> @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) >>>>>> static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) >>>>>> { >>>>>> struct i2c_client *client = to_i2c_client(dev); >>>>>> - uint8_t buffer[len + 1]; >>>>>> + uint8_t buffer[256]; >>>>> You chose to change len to 255, probably because this is a uint8_t but >>>> Correct. >>>> >>>>> this is way too much for this rtc, it only has 10 consecutive registers. >>>> In that case probably the best solution is to add the following line to the >>>> module: >>>> >>>> #define MAX_LEN 10 >>>> >>>> and update the rest of the code as follows: >>>> >>>> uint8_t buffer[MAX_LEN + 1]; >>>> >>> Seems better, yes. >>> >>>>>> buffer[0] = off; >>>>>> memcpy(&buffer[1], data, len); >>>>>> diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c >>>>>> index 79e24ea..00e11c1 100644 >>>>>> --- a/drivers/rtc/rtc-mcp795.c >>>>>> +++ b/drivers/rtc/rtc-mcp795.c >>>>>> @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) >>>>>> { >>>>>> struct spi_device *spi = to_spi_device(dev); >>>>>> int ret; >>>>>> - u8 tx[2 + count]; >>>>>> + u8 tx[257]; >>>> For this particular case it seems to me that the following works just fine: >>>> >>>> #define MAX_COUNT 7 >>>> >>>> u8 tx[MAX_COUNT + 2]; >>>> >>>> What do you think? >>>> >>> Nope, that RTC has a section of 255 bytes that could be read at once so >>> 257 is the correct value. >>> >> I see... >> >> I was looking into this piece of code drivers/rtc/rtc-mcp795.c:302: >> >> tmp[0] = (tmp[0] & 0x80) | bin2bcd(alm->time.tm_sec); >> tmp[1] = (tmp[1] & 0x80) | bin2bcd(alm->time.tm_min); >> tmp[2] = (tmp[2] & 0xE0) | bin2bcd(alm->time.tm_hour); >> tmp[3] = (tmp[3] & 0x80) | bin2bcd(alm->time.tm_wday + 1); >> /* set alarm match: seconds, minutes, hour, day, date and month */ >> tmp[3] |= (MCP795_ALM0C2_BIT | MCP795_ALM0C1_BIT | >> MCP795_ALM0C0_BIT); >> tmp[4] = (tmp[4] & 0xC0) | bin2bcd(alm->time.tm_mday); >> tmp[5] = (tmp[5] & 0xE0) | bin2bcd(alm->time.tm_mon + 1); >> >> ret = mcp795_rtcc_write(dev, MCP795_REG_ALM0_SECONDS, tmp, >> sizeof(tmp)); >> > Yeah, the 128 or 255 bytes NVRAM is not yet supported but this is on my > todo list > I got it. Thanks -- Gustavo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] rtc: remove VLA usage 2018-03-07 21:25 ` Alexandre Belloni 2018-03-07 22:39 ` Gustavo A. R. Silva @ 2018-03-07 22:40 ` Gustavo A. R. Silva 1 sibling, 0 replies; 8+ messages in thread From: Gustavo A. R. Silva @ 2018-03-07 22:40 UTC (permalink / raw) To: Alexandre Belloni, Gustavo A. R. Silva Cc: Alessandro Zummo, linux-rtc, linux-kernel Hi Alexandre, On 03/07/2018 03:25 PM, Alexandre Belloni wrote: > On 07/03/2018 at 14:11:33 -0600, Gustavo A. R. Silva wrote: >> In preparation to enabling -Wvla, remove VLA and replace it >> with a fixed-length array instead. >> > You should probably explain what VLA is and why this is important to do. Sure. I can elaborate a little bit more. >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >> --- >> drivers/rtc/rtc-bq32k.c | 2 +- >> drivers/rtc/rtc-mcp795.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c >> index e8698e9..e4b234a 100644 >> --- a/drivers/rtc/rtc-bq32k.c >> +++ b/drivers/rtc/rtc-bq32k.c >> @@ -74,7 +74,7 @@ static int bq32k_read(struct device *dev, void *data, uint8_t off, uint8_t len) >> static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len) >> { >> struct i2c_client *client = to_i2c_client(dev); >> - uint8_t buffer[len + 1]; >> + uint8_t buffer[256]; >> > You chose to change len to 255, probably because this is a uint8_t but Correct. > this is way too much for this rtc, it only has 10 consecutive registers. In that case probably the best solution is to add the following line to the module: #define MAX_LEN 10 and update the rest of the code as follows: uint8_t buffer[MAX_LEN + 1]; >> buffer[0] = off; >> memcpy(&buffer[1], data, len); >> diff --git a/drivers/rtc/rtc-mcp795.c b/drivers/rtc/rtc-mcp795.c >> index 79e24ea..00e11c1 100644 >> --- a/drivers/rtc/rtc-mcp795.c >> +++ b/drivers/rtc/rtc-mcp795.c >> @@ -82,7 +82,7 @@ static int mcp795_rtcc_write(struct device *dev, u8 addr, u8 *data, u8 count) >> { >> struct spi_device *spi = to_spi_device(dev); >> int ret; >> - u8 tx[2 + count]; >> + u8 tx[257]; For this particular case it seems to me that the following works just fine: #define MAX_COUNT 7 u8 tx[MAX_COUNT + 2]; What do you think? Thanks for your feedback. -- Gustavo >> >> tx[0] = MCP795_WRITE; >> tx[1] = addr; >> -- >> 2.7.4 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-03-07 23:39 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-07 20:11 [PATCH] rtc: remove VLA usage Gustavo A. R. Silva 2018-03-07 21:25 ` Alexandre Belloni 2018-03-07 22:39 ` Gustavo A. R. Silva 2018-03-07 23:01 ` Alexandre Belloni 2018-03-07 23:09 ` Gustavo A. R. Silva 2018-03-07 23:12 ` Alexandre Belloni 2018-03-07 23:14 ` Gustavo A. R. Silva 2018-03-07 22:40 ` Gustavo A. R. Silva
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).