snd_uaudio: Refactor umidi_attach()

[christos]

Summary

Merge umidi_init() with umidi_attach(). Also call umidi_detach() at the
detach label in umidi_attach(), similar to how uaudio_attach() does it.

Sponsored by: The FreeBSD Foundation
MFC after: 1 week

[markj]

From what I can see, now, we initialize the mutex only if sc_midi_chan.valid is true, but we will destroy it unconditionally, so we might destroy an uninitialized mutex.

[christos]

Address Mark's comment: if umidi_attach() fails, set sc->sc_midi_chan.valid to
0, and check that in umidi_detach().

[markj]

sys/dev/sound/usb/uaudio.c

6080

But now we don't do any cleanup, e.g. of fifos, if umidi_attach() fails...

[christos]

sys/dev/sound/usb/uaudio.c

6080

This is a good catch, although the fifo cleanup is not a problem, because the last point of failure in umidi_attach() is the fifo attach, so the only scenario where the fifos are actually attached and need cleanup, will be when umidi_attach() succeeds, in which case umidi_detach() will run normally.

The only thing that can potentially not get cleaned up with this code, is the usbd_transfer_setup().

I will address this.

[christos]

Address comments.

[markj]

sys/dev/sound/usb/uaudio.c

6067

So if umidi_attach() fails, we'll call umidi_detach() here, and then uaudio_attach() will call uaudio_detach(), which calls umidi_detach(). So we'll destroy the mutex twice.

[christos]

sys/dev/sound/usb/uaudio.c

6067

True. I did test error paths to make sure this wouldn't be the case, but I must have missed something, because I didn't indeed hit this error. Now I tried manually making umidi_attach() fail and it does indeed destroy the mutex twice. Will address this now. Sorry for the noise.

[christos]

I will come back to this at some other point, but it might require another series of patches, so abandoning this for now.