Re: [PATCH v2] soundwire: debugfs: move debug statement outside of error handling

[Vinod Koul <vkoul@kernel.org>]

On 21-06-25, 16:09, Rodrigo Gobbi wrote:
> The start_t and finish_t variables are not properly initialized
> if errors happens over request_firmware actions.
> This was also detected by smatch:
> 
> drivers/soundwire/debugfs.c:301 cmd_go() error: uninitialized symbol 'finish_t'.
> drivers/soundwire/debugfs.c:301 cmd_go() error: uninitialized symbol 'start_t'.
> 
> Initialize those variables and move the debug statement outside of
> firmware error handling.
> 
> Signed-off-by: Rodrigo Gobbi <rodrigo.gobbi.7@gmail.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/linux-sound/0db6d0bf-7bac-43a7-b624-a00d3d2bf829@stanley.mountain/
> Fixes: bb5cb09eedce ("soundwire: debugfs: add interface for BPT/BRA transfers")
> ---
> @Vinod Koul, tks for the suggestion. I`ve kept the ktime_t initialization, after moving
> the debug statement, they are not actually needed, but I think there is no harm to keep that.
> Tks and regards.

Thanks, but I guess lets avoid superfluous initialization. we should
drop this as well

> 
> Changelog:
> v2: moving debug statement
> v1: https://lore.kernel.org/linux-sound/20250617015230.54981-1-rodrigo.gobbi.7@gmail.com/#t
> ---
>  drivers/soundwire/debugfs.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/soundwire/debugfs.c b/drivers/soundwire/debugfs.c
> index 3099ea074f10..42d2aba285f5 100644
> --- a/drivers/soundwire/debugfs.c
> +++ b/drivers/soundwire/debugfs.c
> @@ -237,8 +237,8 @@ static int cmd_go(void *data, u64 value)
>  {
>  	const struct firmware *fw = NULL;
>  	struct sdw_slave *slave = data;
> -	ktime_t start_t;
> -	ktime_t finish_t;
> +	ktime_t start_t = 0;
> +	ktime_t finish_t = 0;
>  	int ret;
>  
>  	if (value != 1)
> @@ -291,6 +291,9 @@ static int cmd_go(void *data, u64 value)
>  
>  	finish_t = ktime_get();
>  
> +	dev_dbg(&slave->dev, "command completed, num_byte %zu status %d, time %lld ms\n",
> +		num_bytes, ret, div_u64(finish_t - start_t, NSEC_PER_MSEC));
> +
>  out:
>  	if (fw)
>  		release_firmware(fw);
> @@ -298,9 +301,6 @@ static int cmd_go(void *data, u64 value)
>  	pm_runtime_mark_last_busy(&slave->dev);
>  	pm_runtime_put(&slave->dev);
>  
> -	dev_dbg(&slave->dev, "command completed, num_byte %zu status %d, time %lld ms\n",
> -		num_bytes, ret, div_u64(finish_t - start_t, NSEC_PER_MSEC));
> -
>  	return ret;
>  }
>  DEFINE_DEBUGFS_ATTRIBUTE(cmd_go_fops, NULL,
> -- 
> 2.49.0

-- 
~Vinod