Re: [PATCH i-g-t 5/7] lib/igt_kms.c: refactor parse_path_connector

[Imre Deak <imre.deak@intel.com>]

On Mon, Sep 23, 2024 at 02:55:47AM +0530, Kunal Joshi wrote:
> Use strdup() to make a copy of connector_path
> before passing it to strtok(), so the original
> string remains unmodified
> 
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> ---
>  lib/igt_kms.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 96da33a48..3d2a88b76 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -6670,10 +6670,10 @@ static int parse_path_connector(char *connector_path)
>  {
>  	int connector_id;
>  	char *encoder;
> +	char *connector_path_copy = strdup(connector_path);
>  
> -	encoder = strtok(connector_path, ":");
> +	encoder = strtok(connector_path_copy, ":");
>  	igt_assert_f(!strcmp(encoder, "mst"), "PATH connector property expected to have 'mst'\n");

The copy is leaked, sould be freed. This doesn't look like a refactor,
rather a fix, the commit subject should reflect that. With these fixed:

Reviewed-by: Imre Deak <imre.deak@intel.com>

> -
>  	connector_id = atoi(strtok(NULL, "-"));
>  
>  	return connector_id;
> @@ -6688,13 +6688,11 @@ static int parse_path_connector(char *connector_path)
>  int igt_get_dp_mst_connector_id(igt_output_t *output)
>  {
>  	int connector_id;
> -	char *connector_path;
>  
>  	if (!igt_check_output_is_dp_mst(output))
>  		return -EINVAL;
>  
> -	connector_path = output->config.connector_path;
> -	connector_id = parse_path_connector(connector_path);
> +	connector_id = parse_path_connector(output->config.connector_path);
>  
>  	return connector_id;
>  }
> -- 
> 2.43.0
>