Change in flashrom[master]: cli_classic: Fix Memory leak

[HAOUAS Elyes (Code Review)]

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 
---
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");

[Nico Huber (Code Review)]

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"

[HAOUAS Elyes (Code Review)]

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 
---
M cli_classic.c
1 file changed, 5 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/34405/2

[HAOUAS Elyes (Code Review)]

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

[Nico Huber (Code Review)]

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

[Nico Huber (Code Review)]

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 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/34405
Tested-by: build bot (Jenkins) 
Reviewed-by: Nico Huber 
---
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");