snd_usb_endpoint_free

snd_usb_endpoint_free

[Julia Lawall]

The function snd_usb_endpoint_free in sound/usb/endpoint.c is defined as 
follows:

void snd_usb_endpoint_free(struct list_head *head)
{
        struct snd_usb_endpoint *ep;

        ep = list_entry(head, struct snd_usb_endpoint, list);
        release_urbs(ep, 1);
        kfree(ep);
}

I wonder if the final kfree should be list_del?  In practice, this 
function is only used from snd_usb_audio_disconnect in sound/usb/card.c 
where the entire list is destroyed, but it seems like quite a generic 
function that someone may someday want to use for just freeing one entry.

julia

Re: snd_usb_endpoint_free

[Julia Lawall]

On Wed, 25 Jun 2014, Julia Lawall wrote:

> The function snd_usb_endpoint_free in sound/usb/endpoint.c is defined as 
> follows:
> 
> void snd_usb_endpoint_free(struct list_head *head)
> {
>         struct snd_usb_endpoint *ep;
> 
>         ep = list_entry(head, struct snd_usb_endpoint, list);
>         release_urbs(ep, 1);
>         kfree(ep);
> }
> 
> I wonder if the final kfree should be list_del?  In practice, this 

Sorry, the question should be "I wonder if this function should also use 
list_del", since list_del doesn't subsume kfree.

julia

> function is only used from snd_usb_audio_disconnect in sound/usb/card.c 
> where the entire list is destroyed, but it seems like quite a generic 
> function that someone may someday want to use for just freeing one entry.
> 
> julia
> 

Re: snd_usb_endpoint_free

[Daniel Mack]

Hi Julia,

On 06/25/2014 08:50 AM, Julia Lawall wrote:
> On Wed, 25 Jun 2014, Julia Lawall wrote:
>> The function snd_usb_endpoint_free in sound/usb/endpoint.c is defined as 
>> follows:
>>
>> void snd_usb_endpoint_free(struct list_head *head)
>> {
>>         struct snd_usb_endpoint *ep;
>>
>>         ep = list_entry(head, struct snd_usb_endpoint, list);
>>         release_urbs(ep, 1);
>>         kfree(ep);
>> }
>>
>> I wonder if the final kfree should be list_del?  In practice, this 
> 
> Sorry, the question should be "I wonder if this function should also use 
> list_del", since list_del doesn't subsume kfree.
> 
> julia
> 
>> function is only used from snd_usb_audio_disconnect in sound/usb/card.c 
>> where the entire list is destroyed, but it seems like quite a generic 
>> function that someone may someday want to use for just freeing one entry.

Jup, you're right, a list_del() in there wouldn't harm. However, it
currently wouldn't buy us anything either, and other functions we call
from snd_usb_audio_disconnect() for list members don't kill their own
list entry node themselves.


Thanks,
Daniel

Re: snd_usb_endpoint_free

[Julia Lawall]

On Wed, 25 Jun 2014, Daniel Mack wrote:

> Hi Julia,
>
> On 06/25/2014 08:50 AM, Julia Lawall wrote:
> > On Wed, 25 Jun 2014, Julia Lawall wrote:
> >> The function snd_usb_endpoint_free in sound/usb/endpoint.c is defined as
> >> follows:
> >>
> >> void snd_usb_endpoint_free(struct list_head *head)
> >> {
> >>         struct snd_usb_endpoint *ep;
> >>
> >>         ep = list_entry(head, struct snd_usb_endpoint, list);
> >>         release_urbs(ep, 1);
> >>         kfree(ep);
> >> }
> >>
> >> I wonder if the final kfree should be list_del?  In practice, this
> >
> > Sorry, the question should be "I wonder if this function should also use
> > list_del", since list_del doesn't subsume kfree.
> >
> > julia
> >
> >> function is only used from snd_usb_audio_disconnect in sound/usb/card.c
> >> where the entire list is destroyed, but it seems like quite a generic
> >> function that someone may someday want to use for just freeing one entry.
>
> Jup, you're right, a list_del() in there wouldn't harm. However, it
> currently wouldn't buy us anything either, and other functions we call
> from snd_usb_audio_disconnect() for list members don't kill their own
> list entry node themselves.

OK, thanks for the clarification.

julia

Re: snd_usb_endpoint_free

[Takashi Iwai]

At Wed, 25 Jun 2014 12:13:25 +0200,
Daniel Mack wrote:
> 
> Hi Julia,
> 
> On 06/25/2014 08:50 AM, Julia Lawall wrote:
> > On Wed, 25 Jun 2014, Julia Lawall wrote:
> >> The function snd_usb_endpoint_free in sound/usb/endpoint.c is defined as 
> >> follows:
> >>
> >> void snd_usb_endpoint_free(struct list_head *head)
> >> {
> >>         struct snd_usb_endpoint *ep;
> >>
> >>         ep = list_entry(head, struct snd_usb_endpoint, list);
> >>         release_urbs(ep, 1);
> >>         kfree(ep);
> >> }
> >>
> >> I wonder if the final kfree should be list_del?  In practice, this 
> > 
> > Sorry, the question should be "I wonder if this function should also use 
> > list_del", since list_del doesn't subsume kfree.
> > 
> > julia
> > 
> >> function is only used from snd_usb_audio_disconnect in sound/usb/card.c 
> >> where the entire list is destroyed, but it seems like quite a generic 
> >> function that someone may someday want to use for just freeing one entry.
> 
> Jup, you're right, a list_del() in there wouldn't harm. However, it
> currently wouldn't buy us anything either, and other functions we call
> from snd_usb_audio_disconnect() for list members don't kill their own
> list entry node themselves.

Yes, it'd be better but the current code works as is, too.

In anyway, I'm going to add a fix later; we're working on a
disconnection race fix that involves with this function.


thanks,

Takashi