All of lore.kernel.org
 help / color / mirror / Atom feed
* [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: 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
* [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
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.