[U-Boot] [PATCH] common: cli_hush: avoid memory leak
Need to free memory avoid memory leak, when error.
Signed-off-by: Peng Fan Peng.Fan@freescale.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com --- common/cli_hush.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/common/cli_hush.c b/common/cli_hush.c index f075459..ab85225 100644 --- a/common/cli_hush.c +++ b/common/cli_hush.c @@ -2474,8 +2474,10 @@ static int done_word(o_string *dest, struct p_context *ctx) if (child->argv == NULL) return 1; child->argv_nonnull = realloc(child->argv_nonnull, (argc+1)*sizeof(*child->argv_nonnull)); - if (child->argv_nonnull == NULL) + if (child->argv_nonnull == NULL) { + free(str); return 1; + } child->argv[argc-1]=str; child->argv_nonnull[argc-1] = dest->nonnull; child->argv[argc]=NULL;
"enable" is unsigned char type and its value will not be negative, so discard "enable < 0".
Signed-off-by: Peng Fan Peng.Fan@freescale.com Cc: Diego Santa Cruz Diego.SantaCruz@spinetix.com Cc: Pantelis Antoniou pantelis.antoniou@konsulko.com Cc: Andrew Gabbasov andrew_gabbasov@mentor.com Cc: Simon Glass sjg@chromium.org Cc: Stefano Babic sbabic@denx.de Cc: Tom Rini trini@konsulko.com --- common/cmd_mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/cmd_mmc.c b/common/cmd_mmc.c index dfc1ec8..a6b7313 100644 --- a/common/cmd_mmc.c +++ b/common/cmd_mmc.c @@ -747,7 +747,7 @@ static int do_mmc_rst_func(cmd_tbl_t *cmdtp, int flag, dev = simple_strtoul(argv[1], NULL, 10); enable = simple_strtoul(argv[2], NULL, 10);
- if (enable > 2 || enable < 0) { + if (enable > 2) { puts("Invalid RST_n_ENABLE value\n"); return CMD_RET_USAGE; }
Hi Peng,
On 25 November 2015 at 01:16, Peng Fan Peng.Fan@freescale.com wrote:
"enable" is unsigned char type and its value will not be negative, so discard "enable < 0".
Signed-off-by: Peng Fan Peng.Fan@freescale.com Cc: Diego Santa Cruz Diego.SantaCruz@spinetix.com Cc: Pantelis Antoniou pantelis.antoniou@konsulko.com Cc: Andrew Gabbasov andrew_gabbasov@mentor.com Cc: Simon Glass sjg@chromium.org Cc: Stefano Babic sbabic@denx.de Cc: Tom Rini trini@konsulko.com
common/cmd_mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
Even better if this variable changed to uint, instead of u8.
Regards, Simon
Hi Simon, On Thu, Nov 26, 2015 at 06:51:58PM -0800, Simon Glass wrote:
Hi Peng,
On 25 November 2015 at 01:16, Peng Fan Peng.Fan@freescale.com wrote:
"enable" is unsigned char type and its value will not be negative, so discard "enable < 0".
Signed-off-by: Peng Fan Peng.Fan@freescale.com Cc: Diego Santa Cruz Diego.SantaCruz@spinetix.com Cc: Pantelis Antoniou pantelis.antoniou@konsulko.com Cc: Andrew Gabbasov andrew_gabbasov@mentor.com Cc: Simon Glass sjg@chromium.org Cc: Stefano Babic sbabic@denx.de Cc: Tom Rini trini@konsulko.com
common/cmd_mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
Even better if this variable changed to uint, instead of u8.
Thanks for reviewing, But I prefer to let it be, since mmc_set_rst_n_function takes u8 type for input parameter.
Thanks, Peng.
Regards, Simon
--
On Wed, Nov 25, 2015 at 05:16:21PM +0800, Peng Fan wrote:
"enable" is unsigned char type and its value will not be negative, so discard "enable < 0".
Signed-off-by: Peng Fan Peng.Fan@freescale.com Cc: Diego Santa Cruz Diego.SantaCruz@spinetix.com Cc: Pantelis Antoniou pantelis.antoniou@konsulko.com Cc: Andrew Gabbasov andrew_gabbasov@mentor.com Cc: Simon Glass sjg@chromium.org Cc: Stefano Babic sbabic@denx.de Cc: Tom Rini trini@konsulko.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
Hi Peng,
On 25 November 2015 at 02:16, Peng Fan Peng.Fan@freescale.com wrote:
Need to free memory avoid memory leak, when error.
Signed-off-by: Peng Fan Peng.Fan@freescale.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
common/cli_hush.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/common/cli_hush.c b/common/cli_hush.c index f075459..ab85225 100644 --- a/common/cli_hush.c +++ b/common/cli_hush.c @@ -2474,8 +2474,10 @@ static int done_word(o_string *dest, struct p_context *ctx) if (child->argv == NULL) return 1; child->argv_nonnull = realloc(child->argv_nonnull, (argc+1)*sizeof(*child->argv_nonnull));
if (child->argv_nonnull == NULL)
if (child->argv_nonnull == NULL) {free(str); return 1;} child->argv[argc-1]=str; child->argv_nonnull[argc-1] = dest->nonnull; child->argv[argc]=NULL;-- 2.6.2
Reviewed-by: Simon Glass sjg@chromium.org
It looks like there is another memory leak at the 'return 1' immediately above.
Regards, Simon
Hi Simon, On Thu, Nov 26, 2015 at 09:49:38AM -0800, Simon Glass wrote:
Hi Peng,
On 25 November 2015 at 02:16, Peng Fan Peng.Fan@freescale.com wrote:
Need to free memory avoid memory leak, when error.
Signed-off-by: Peng Fan Peng.Fan@freescale.com Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
common/cli_hush.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/common/cli_hush.c b/common/cli_hush.c index f075459..ab85225 100644 --- a/common/cli_hush.c +++ b/common/cli_hush.c @@ -2474,8 +2474,10 @@ static int done_word(o_string *dest, struct p_context *ctx) if (child->argv == NULL) return 1; child->argv_nonnull = realloc(child->argv_nonnull, (argc+1)*sizeof(*child->argv_nonnull));
if (child->argv_nonnull == NULL)
if (child->argv_nonnull == NULL) {free(str); return 1;} child->argv[argc-1]=str; child->argv_nonnull[argc-1] = dest->nonnull; child->argv[argc]=NULL;-- 2.6.2
Reviewed-by: Simon Glass sjg@chromium.org
It looks like there is another memory leak at the 'return 1' immediately above.
My coverity check tool does not report this (:-, but seems this is true memory leak if child->argv is NULL. Thanks for pointing this out. I'll fix this in V2.
Thanks, Peng.
Regards, Simon
--
participants (4)
-
Peng Fan -
Peng Fan -
Simon Glass -
Tom Rini