Re: [PATCH] sound: sof: ioc4-topology: avoid extra dai_params copy

["Arnd Bergmann" <arnd@xxxxxxxx>]

On Wed, Aug 7, 2024, at 17:09, Mark Brown wrote:
> On Wed, Aug 07, 2024 at 10:02:27AM +0200, Arnd Bergmann wrote:
>
>> From what I can tell, this was unintentional, as both
>> sof_ipc4_prepare_dai_copier() and sof_ipc4_prepare_copier_module() make a
>> copy for the same purpose, but copying it once has the exact same effect.
>
>> Remove the extra copy and change the direct struct assignment to
>> an explicit memcpy() call to make it clearer to the reader that this
>> is what happens. Note that gcc treats struct assignment as a memcpy()
>> that may be inlined anyway, so the resulting object code is the same.
>
> The effect of the copy is to ensure that if the function fails the
> argument is unmodified - did you do the analysis to check that it's OK
> to modify on error?  Your commit log says "the same purpose" but never
> specifies what that purpose is.

There is always a chance that I misunderstood the code, but
yes, I did understand that the idea is to not modify the
parameters inside of sof_ipc4_prepare_dai_copier.

The only caller is in sof_ipc4_prepare_copier_module(), which
achieves the exact same bit by doing the same:

         /*
          * Use the fe_params as a base for the copier configuration.
          * The ref_params might get updated to reflect what format is
          * supported by the copier on the DAI side.
          *
          * In case of capture the ref_params returned will be used to
          * find the input configuration of the copier.
          */
         memcpy(&ref_params, fe_params, sizeof(ref_params));
         ret = sof_ipc4_prepare_dai_copier(sdev, dai, &ref_params, dir);
         if (ret < 0)
                  return ret;

So when sof_ipc4_prepare_dai_copier() fails, the caller's
local 'ref_params' structure is no longer used anywhere.

     Arnd