Re: [PATCH] rtc: remove VLA usage
From: Gustavo A. R. Silva
Date: Wed Mar 07 2018 - 18:39:09 EST
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@xxxxxxxxxxxxxx>
---
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