On Sun, 03 Sep 2023 17:04:47 +0200, Christophe JAILLET wrote:
Le 03/09/2023 à 16:23, Takashi Iwai a écrit :
On Sun, 03 Sep 2023 15:06:00 +0200, Christophe JAILLET wrote:
If usb_alloc_coherent() or usb_urb_ep_type_check() fail, we should release the resources previously allocated.
Those are freed in the caller side, start_input_streams() instead.
Thanks for the fast review.
Hmpm, If IIUC, resources allocated *before* the ending "ep->num_urbs++" still need to be freed here, otherwise free_midi_urbs() in the caller will not free them.
Do you agree?
If yes, I can send v2 which would look like: usb_alloc_urb() if (err) return -ENOMEM
usb_alloc_coherent() if (err) { usb_free_urb() urb = NULL return -ENOMEM }
usb_urb_ep_type_check()if (err) { usb_free_coherent() usb_free_urb() urb = NULL return -err }
Or, if yuo prefer, with an error handling path just like below, but without the final free_midi_urbs() + a comment explaining that the caller does this part of job instead.
Indeed. The fix would be rather a oneliner like below, though:
--- a/sound/usb/midi2.c +++ b/sound/usb/midi2.c @@ -265,7 +265,7 @@ static void free_midi_urbs(struct snd_usb_midi2_endpoint *ep)
if (!ep) return; - for (i = 0; i < ep->num_urbs; ++i) { + for (i = 0; i < NUM_URBS; ++i) { ctx = &ep->urbs[i]; if (!ctx->urb) break;
That was the intended behavior of free_midi_urbs().
Takashi