Re: [PATCH 15/78] ASoC: codecs: cs42l43: Use guard() for mutex locks

[Takashi Iwai <tiwai@suse.de>]

On Fri, 19 Jun 2026 12:57:57 +0200,
Bui Duc Phuc wrote:
> 
> Hi Charles, David,
> 
> 
> 
> > > > > > I believe you have to use scoped_guard here, as there is a return
> > > > > > from the function above, if memory serves it attempts to release
> > > > > > the mutex on that path despite it being above the guard.
> > > > >
> > > > > Indeed.
> > > > > I believe clang will complain.
> > > > > That makes these mechanical conversions of existing code dangerous churn.
> > > > >
> > > > > While using guard() (etc) can make it easier to ensure the lock is released
> > > > > when functions have multiple error exits, I'm not convinced it makes the
> > > > > code any easier to read (other people may disagree).
> > > >
> > > > I built the code with both GCC and Clang and didn't see any warnings.
> > > >
> > > > My understanding was that the early return exits the function before
> > > > the guard is instantiated, so it should not affect the guard's cleanup
> > > > handling.
> > >
> > > When a variable is defined (and initialised) part way down a block the
> > > compiler moves the definition to the top of the block but doesn't initialise
> > > it at all, the first assignment happens where the code contains the
> > > definition.
> > >
> > > However the destructor is always called at the end of the block.
> > > So if you return from a function before the definition the destructor
> > > is called with an uninitialised argument.
> >
> > My understanding was exactly as your David, but it seems that isn't
> > the whole story and indeed I had to fix a bug in our SDCA code
> > that hit this.  However testing this out, results in some things I
> > find very hard to explain.
> >
> > It seems as far as I have managed to test, the code below works
> > fine as Phuc suggests. It does not appear to run the mutex_unlock
> > on the error path.
> >
> > int function()
> > {
> >         if (error)
> >                 return;
> >
> >         guard(mutex)(&mutex);
> >
> >         stuff();
> >
> >         return;
> > }
> >
> 
> Thanks both for the clarification.
> 
> > The situation I hit this in before that doesn't work was actually
> > this:
> >
> > int function()
> > {
> >         if (error)
> >                 goto error_label;
> >
> >         guard(mutex)(&mutex);
> >
> >         stuff();
> >
> > error_label;
> >         return;
> > }
> >
> > Which in this case it does run the mutex_unlock and NULL pointer.
> > Will try to find sometime to look at the generated assembly, but
> > this basically totally blows my mind. Very unclear as to if this
> > is supposed to work this way or just does by pure luck.
> >
> 
> As stated in cleanup.h, mixing goto-based cleanup and scope-based
> cleanup helpers in the same function is not expected, so I think
> we should keep a consistent approach here.

Right, and IIRC, clang would complain the mixed goto case at least
with W=1.


Takashi