Re: [PATCH v2 3/4] ASoC: Intel: atom: Use scoped_guard() for scoped locking

[Bui Duc Phuc]

Hi Cezary,

Thanks for the review.

> > +     struct sst_runtime_stream *stream __free(kfree) = NULL;
>
> No mixing of cleanup classes.  Cleaning locking != cleaning pointers.
> One action per patch.
>
> ...
>
> > @@ -347,13 +346,10 @@ static int sst_media_open(struct snd_pcm_substream *substream,
> >       snd_pcm_hw_constraint_step(substream->runtime, 0,
> >                          SNDRV_PCM_HW_PARAM_PERIODS, 2);
> >
> > +     stream = NULL;
> > +
>
> Is this needed?  If you believe so, then I'd rather see it placed
> immediately after any operation that could cause the variable to leave
> the scope.
>

I agree with your point. Here is how I will address them in V3 submission.

1. About splitting the patch :
I will split this change into a 2-patch for v3:
Patch 1 :  Use scoped_guard() for scoped locking
Patch 2 :  Use __free(kfree) for stream pointer

2. About the "stream = NULL" assignment:
Yes, this is strictly necessary. Since 'stream' is managed by
__free(kfree), setting it to NULL before a successful return
ensures that the cleanup helper does not free the allocated memory
upon function exit. The deallocation will be handled
later by sst_media_close().
Based on your suggestion regarding the scope, I will move this
assignment to be placed immediately after the final constraint
operation where the variable leaves its effective scope.

- stream = NULL;
- return snd_pcm_hw_constraint_integer(runtime,

SNDRV_PCM_HW_PARAM_PERIODS);
+ ret_val = snd_pcm_hw_constraint_integer(runtime,

SNDRV_PCM_HW_PARAM_PERIODS);
+ if (ret_val < 0)
+         return ret_val;

+ stream = NULL;

+ return ret_val;

Does this alignment look good to you ?

Best regards,
Phuc