* [U-Boot] [PATCH] common: cli_hush: avoid memory leak
@ 2015-11-25 9:16 Peng Fan
2015-11-25 9:16 ` [U-Boot] [PATCH] common: mmc: unsigned char compared against 0 Peng Fan
2015-11-26 17:49 ` [U-Boot] [PATCH] common: cli_hush: avoid memory leak Simon Glass
0 siblings, 2 replies; 7+ messages in thread
From: Peng Fan @ 2015-11-25 9:16 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [U-Boot] [PATCH] common: mmc: unsigned char compared against 0
2015-11-25 9:16 [U-Boot] [PATCH] common: cli_hush: avoid memory leak Peng Fan
@ 2015-11-25 9:16 ` Peng Fan
2015-11-27 2:51 ` Simon Glass
2015-12-06 22:06 ` [U-Boot] " Tom Rini
2015-11-26 17:49 ` [U-Boot] [PATCH] common: cli_hush: avoid memory leak Simon Glass
1 sibling, 2 replies; 7+ messages in thread
From: Peng Fan @ 2015-11-25 9:16 UTC (permalink / raw)
To: u-boot
"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;
}
--
2.6.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [U-Boot] [PATCH] common: mmc: unsigned char compared against 0
2015-11-25 9:16 ` [U-Boot] [PATCH] common: mmc: unsigned char compared against 0 Peng Fan
@ 2015-11-27 2:51 ` Simon Glass
2015-11-27 4:58 ` Peng Fan
2015-12-06 22:06 ` [U-Boot] " Tom Rini
1 sibling, 1 reply; 7+ messages in thread
From: Simon Glass @ 2015-11-27 2:51 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] common: mmc: unsigned char compared against 0
2015-11-27 2:51 ` Simon Glass
@ 2015-11-27 4:58 ` Peng Fan
0 siblings, 0 replies; 7+ messages in thread
From: Peng Fan @ 2015-11-27 4:58 UTC (permalink / raw)
To: u-boot
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
--
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] common: mmc: unsigned char compared against 0
2015-11-25 9:16 ` [U-Boot] [PATCH] common: mmc: unsigned char compared against 0 Peng Fan
2015-11-27 2:51 ` Simon Glass
@ 2015-12-06 22:06 ` Tom Rini
1 sibling, 0 replies; 7+ messages in thread
From: Tom Rini @ 2015-12-06 22:06 UTC (permalink / raw)
To: u-boot
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!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151206/c0c0ab22/attachment.sig>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] common: cli_hush: avoid memory leak
2015-11-25 9:16 [U-Boot] [PATCH] common: cli_hush: avoid memory leak Peng Fan
2015-11-25 9:16 ` [U-Boot] [PATCH] common: mmc: unsigned char compared against 0 Peng Fan
@ 2015-11-26 17:49 ` Simon Glass
2015-11-27 2:04 ` Peng Fan
1 sibling, 1 reply; 7+ messages in thread
From: Simon Glass @ 2015-11-26 17:49 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread* [U-Boot] [PATCH] common: cli_hush: avoid memory leak
2015-11-26 17:49 ` [U-Boot] [PATCH] common: cli_hush: avoid memory leak Simon Glass
@ 2015-11-27 2:04 ` Peng Fan
0 siblings, 0 replies; 7+ messages in thread
From: Peng Fan @ 2015-11-27 2:04 UTC (permalink / raw)
To: u-boot
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
--
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-12-06 22:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-25 9:16 [U-Boot] [PATCH] common: cli_hush: avoid memory leak Peng Fan
2015-11-25 9:16 ` [U-Boot] [PATCH] common: mmc: unsigned char compared against 0 Peng Fan
2015-11-27 2:51 ` Simon Glass
2015-11-27 4:58 ` Peng Fan
2015-12-06 22:06 ` [U-Boot] " Tom Rini
2015-11-26 17:49 ` [U-Boot] [PATCH] common: cli_hush: avoid memory leak Simon Glass
2015-11-27 2:04 ` Peng Fan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.