Re: [PATCH i-g-t v2 3/7] runner/settings: Drop extra strdup

[Gustavo Sousa <gustavo.sousa@intel.com>]

Quoting Lucas De Marchi (2025-01-23 03:23:46-03:00)
>On Wed, Jan 22, 2025 at 08:34:21AM -0300, Gustavo Sousa wrote:
>>Quoting Lucas De Marchi (2025-01-21 19:57:29-03:00)
>>>No need to strdup() again since the fscanf() function is already
>>>allocating the variable. Just set the pointer to NULL so we "leak" our
>>>variable to be saved in the settings.
>>>
>>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>>---
>>> runner/settings.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>>diff --git a/runner/settings.c b/runner/settings.c
>>>index 13694a51c..96377f1de 100644
>>>--- a/runner/settings.c
>>>+++ b/runner/settings.c
>>>@@ -1164,7 +1164,11 @@ static unsigned long parse_ul(char **pval)
>>>
>>> static char *parse_str(char **pval)
>>> {
>>>-        return *pval ? strdup(*pval) : NULL;
>>>+        char *ret = *pval;
>>>+
>>>+        *pval = NULL;
>>
>>Okay. This explains the need for char **pval in the previous patch.
>>Maybe a heads up in #2's commit message would clarify things :-)
>
>yeah, sorry. What about naming it "leak_str()"?

I think parse_str() is okay.

I just really missed a heads up in the previous commit message that
using a char ** was being done for an upcoming change.

Or even, you could change it to char ** in this patch.

--
Gustavo Sousa

>
>Lucas De Marchi
>
>>
>>Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
>>
>>>+
>>>+        return ret;
>>> }
>>>
>>> #define PARSE_LINE(s, name, val, field, _f)        \
>>>--
>>>2.48.0
>>>