From bb9b335e31cda78f4a0e7c3e546ffcc1499c989b Mon Sep 17 00:00:00 2001 From: Randall Spangler Date: Thu, 7 Nov 2013 13:18:48 -0800 Subject: [PATCH] stm32: Don't use a stack buffer for i2c_read_string() We read a counted string (byte 0 = count, bytes 1 - count = chars) and convert it to a null-terminated string. Since both have a 1-byte overhead, we can use the destination buffer instead of using a stack-based buffer. BUG=chrome-os-partner:23928 BRANCH=none (pit is affected, but battery console command isn't used on end user systems) TEST=battery command shows correct strings (SDI / 4302D40 / LiP), and doesn't stack overflow. Change-Id: Ic0f111cde2d57b41d6ce9287e0c771acc09a8869 Signed-off-by: Randall Spangler Reviewed-on: https://chromium-review.googlesource.com/176116 Reviewed-by: Bill Richardson --- chip/stm32/i2c-stm32f.c | 15 +++++++-------- chip/stm32/i2c-stm32l.c | 17 ++++++++--------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/chip/stm32/i2c-stm32f.c b/chip/stm32/i2c-stm32f.c index 52b56c363a..9fba464456 100644 --- a/chip/stm32/i2c-stm32f.c +++ b/chip/stm32/i2c-stm32f.c @@ -874,25 +874,24 @@ int i2c_read_string(int port, int slave_addr, int offset, uint8_t *data, /* * TODO(crosbug.com/p/23569): when i2c_xfer() supports start/stop bits, - * won't need a temp buffer, and this code can merge with the LM4 - * implementation and move to i2c_common.c. + * merge this with the LM4 implementation and move to i2c_common.c. */ - uint8_t buffer[SMBUS_MAX_BLOCK + 1]; if ((len <= 0) || (len > SMBUS_MAX_BLOCK)) return EC_ERROR_INVAL; i2c_lock(port, 1); + /* Read the counted string into the output buffer */ reg = offset; - rv = i2c_xfer(port, slave_addr, ®, 1, buffer, SMBUS_MAX_BLOCK + 1, - I2C_XFER_SINGLE); + rv = i2c_xfer(port, slave_addr, ®, 1, data, len, I2C_XFER_SINGLE); if (rv == EC_SUCCESS) { /* Block length is the first byte of the returned buffer */ - block_length = MIN(buffer[0], len - 1); - buffer[block_length + 1] = 0; + block_length = MIN(data[0], len - 1); - memcpy(data, buffer+1, block_length + 1); + /* Move data down, then null-terminate it */ + memmove(data, data + 1, block_length); + data[block_length] = 0; } i2c_lock(port, 0); diff --git a/chip/stm32/i2c-stm32l.c b/chip/stm32/i2c-stm32l.c index 6ff355c0ff..c05ddd980f 100644 --- a/chip/stm32/i2c-stm32l.c +++ b/chip/stm32/i2c-stm32l.c @@ -459,26 +459,25 @@ int i2c_read_string(int port, int slave_addr, int offset, uint8_t *data, uint8_t reg, block_length; /* - * TODO(crosbug.com/p/23569): When i2c_xfer() supports start/stop bits - * on all platforms, won't need a temp buffer, and this code can merge - * with the LM4 implementation and move to i2c_common.c. + * TODO(crosbug.com/p/23569): when i2c_xfer() supports start/stop bits, + * merge this with the LM4 implementation and move to i2c_common.c. */ - uint8_t buffer[SMBUS_MAX_BLOCK + 1]; if ((len <= 0) || (len > SMBUS_MAX_BLOCK)) return EC_ERROR_INVAL; i2c_lock(port, 1); + /* Read the counted string into the output buffer */ reg = offset; - rv = i2c_xfer(port, slave_addr, ®, 1, buffer, SMBUS_MAX_BLOCK + 1, - I2C_XFER_SINGLE); + rv = i2c_xfer(port, slave_addr, ®, 1, data, len, I2C_XFER_SINGLE); if (rv == EC_SUCCESS) { /* Block length is the first byte of the returned buffer */ - block_length = MIN(buffer[0], len - 1); - buffer[block_length + 1] = 0; + block_length = MIN(data[0], len - 1); - memcpy(data, buffer+1, block_length + 1); + /* Move data down, then null-terminate it */ + memmove(data, data + 1, block_length); + data[block_length] = 0; } i2c_lock(port, 0);