Change in flashrom[master]: cli_classic: Fix Memory leak
HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/34405 ) Change subject: cli_classic: Fix Memory leak ...................................................................... cli_classic: Fix Memory leak Found-by scan-build: 7.0.1-8 Change-Id: I84e642b57b95953f376569e443ef8d8eda7bf98f Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> --- M cli_classic.c 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/34405/1 diff --git a/cli_classic.c b/cli_classic.c index b79f953..19d65cc 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -383,6 +383,11 @@ fprintf(stderr, "Log file not supported in standalone mode. Aborting.\n"); cli_classic_abort_usage(); #else /* STANDALONE */ + if (logfile[0] != '\0') { + fprintf(stderr, "Warning: log file not free of older string. Running free() rightnow.\n"); + free(logfile); + } + logfile = strdup(optarg); if (logfile[0] == '\0') { fprintf(stderr, "No log filename specified.\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/34405 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I84e642b57b95953f376569e443ef8d8eda7bf98f Gerrit-Change-Number: 34405 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-MessageType: newchange
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34405 ) Change subject: cli_classic: Fix Memory leak ...................................................................... Patch Set 1: (2 comments) Looks good, but the output should be more user friendly. https://review.coreboot.org/c/flashrom/+/34405/1/cli_classic.c File cli_classic.c: https://review.coreboot.org/c/flashrom/+/34405/1/cli_classic.c@386 PS1, Line 386: logfile[0] != '\0' nit, just `if (logfile)` would be enough. It was initialized to `NULL` above, `if (logfile)` would tell us if that changed already. https://review.coreboot.org/c/flashrom/+/34405/1/cli_classic.c@387 PS1, Line 387: Warning: log file not free of older string. Running free() rightnow.\n Well, if we follow the code flow, it's clear that this happens (only) when there were multiple `-o <logfile>` on the command line (you can try btw.). So we should state that here (a user wouldn't understand what free() means). e.g. "Warning: -o/--output specified multiple times.\n" -- To view, visit https://review.coreboot.org/c/flashrom/+/34405 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I84e642b57b95953f376569e443ef8d8eda7bf98f Gerrit-Change-Number: 34405 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Comment-Date: Thu, 18 Jul 2019 13:29:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/34405 to look at the new patch set (#2). Change subject: cli_classic: Fix Memory leak ...................................................................... cli_classic: Fix Memory leak Found-by: scan-build 7.0.1-8 Change-Id: I84e642b57b95953f376569e443ef8d8eda7bf98f Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> --- M cli_classic.c 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/34405/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/34405 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I84e642b57b95953f376569e443ef8d8eda7bf98f Gerrit-Change-Number: 34405 Gerrit-PatchSet: 2 Gerrit-Owner: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-MessageType: newpatchset
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34405 ) Change subject: cli_classic: Fix Memory leak ...................................................................... Patch Set 2: (2 comments) Thank you https://review.coreboot.org/c/flashrom/+/34405/1/cli_classic.c File cli_classic.c: https://review.coreboot.org/c/flashrom/+/34405/1/cli_classic.c@386 PS1, Line 386: logfile[0] != '\0'
nit, just `if (logfile)` would be enough. It was initialized to `NULL` above, […] Done. Thank you.
https://review.coreboot.org/c/flashrom/+/34405/1/cli_classic.c@387 PS1, Line 387: Warning: log file not free of older string. Running free() rightnow.\n
Well, if we follow the code flow, it's clear that this happens (only) when […] Done, Thx
-- To view, visit https://review.coreboot.org/c/flashrom/+/34405 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I84e642b57b95953f376569e443ef8d8eda7bf98f Gerrit-Change-Number: 34405 Gerrit-PatchSet: 2 Gerrit-Owner: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Comment-Date: Thu, 18 Jul 2019 15:34:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34405 ) Change subject: cli_classic: Fix Memory leak ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/34405 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I84e642b57b95953f376569e443ef8d8eda7bf98f Gerrit-Change-Number: 34405 Gerrit-PatchSet: 2 Gerrit-Owner: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Thu, 18 Jul 2019 23:45:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/flashrom/+/34405 ) Change subject: cli_classic: Fix Memory leak ...................................................................... cli_classic: Fix Memory leak Found-by: scan-build 7.0.1-8 Change-Id: I84e642b57b95953f376569e443ef8d8eda7bf98f Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/flashrom/+/34405 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Nico Huber <nico.h@gmx.de> --- M cli_classic.c 1 file changed, 5 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/cli_classic.c b/cli_classic.c index b79f953..0591bfe 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -383,6 +383,11 @@ fprintf(stderr, "Log file not supported in standalone mode. Aborting.\n"); cli_classic_abort_usage(); #else /* STANDALONE */ + if (logfile) { + fprintf(stderr, "Warning: -o/--output specified multiple times.\n"); + free(logfile); + } + logfile = strdup(optarg); if (logfile[0] == '\0') { fprintf(stderr, "No log filename specified.\n"); -- To view, visit https://review.coreboot.org/c/flashrom/+/34405 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I84e642b57b95953f376569e443ef8d8eda7bf98f Gerrit-Change-Number: 34405 Gerrit-PatchSet: 3 Gerrit-Owner: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: merged
participants (2)
-
HAOUAS Elyes (Code Review) -
Nico Huber (Code Review)