[PATCH i-g-t v2 5/7] runner: Free settings at the end

[Lucas De Marchi]

On Thu, Jan 23, 2025 at 12:28:28AM -0600, Lucas De Marchi wrote:
>On Wed, Jan 22, 2025 at 08:46:33AM -0300, Gustavo Sousa wrote:
>>Quoting Lucas De Marchi (2025-01-21 19:57:31-03:00)
>>>Keep valgrind happy with the normal allocations so we can find the real
>>>leaks. Avoid this kind of leak:
>>>
>>>       ==806592== 8 bytes in 1 blocks are definitely lost in loss record 46 of 188
>>>       ==806592==    at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
>>>       ==806592==    by 0x4BD534E: strdup (strdup.c:42)
>>>       ==806592==    by 0x10E6AA: parse_options (settings.c:863)
>>>       ==806592==    by 0x10D2DD: main (runner.c:19)
>>>
>>>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>
>>This looks correct, so:
>>
>>Reviewed-by: Gustavo Sousa <gustavo.sousa at intel.com>
>>
>>, but it seems we also need to make initialize_execute_state() and
>>initialize_execute_state_from_resume() properly clear_settings() when
>>things go wrong internally.
>
>Looking at runner/runner.c, there are more places to do it when things
>go bad, example create_job_list(). Maybe replace the returns on error
>path with `goto cleanup`?

well... that will need more work since some of the calls do cleanup of
things they didn't initialize. Example
initialize_execute_state_from_resume() closing the dirfd that is passed
in. I updated the message stating that it only fixes the leak on success
and that error paths will need some more changes in future.

thanks
Lucas De Marchi

>
>Lucas De Marchi
>
>>
>>--
>>Gustavo Sousa
>>
>>>---
>>>runner/resume.c | 2 ++
>>>runner/runner.c | 2 ++
>>>2 files changed, 4 insertions(+)
>>>
>>>diff --git a/runner/resume.c b/runner/resume.c
>>>index 0f4e42bfa..ed17351c6 100644
>>>--- a/runner/resume.c
>>>+++ b/runner/resume.c
>>>@@ -55,6 +55,8 @@ int main(int argc, char **argv)
>>>                exitcode = 3;
>>>        }
>>>
>>>+        clear_settings(&settings);
>>>+
>>>        printf("Done.\n");
>>>        return exitcode;
>>>}
>>>diff --git a/runner/runner.c b/runner/runner.c
>>>index 4855ad641..258b30b36 100644
>>>--- a/runner/runner.c
>>>+++ b/runner/runner.c
>>>@@ -49,6 +49,8 @@ int main(int argc, char **argv)
>>>                exitcode = 1;
>>>        }
>>>
>>>+        clear_settings(&settings);
>>>+
>>>        printf("Done.\n");
>>>        return exitcode;
>>>}
>>>--
>>>2.48.0
>>>