On Thu, 31 Mar 2016 16:03:55 +0200, Vladis Dronov wrote:
Hello, Takashi, all,
No, it has nothing to do with the double-free bug itself. Such an optimization shouldn't be put in a fix patch
This piece of code move alone fixes the double-free bug in create_fixed_stream_quirk(), so I believe it is related.
The merits you pointed are irrelevant from the double-free bug.
Besides, a lot of stuff is created and initialized in snd_usb_add_audio_stream() and while I do not see another use-after-free bug, it could be there. By moving this code we avoid these potential bugs we have not hit yet.
It's a different issue. The only judgment now is which one is clearer to understand. The code efficiency isn't a question for this bug, since the error condition is very rare, and it's no hot path, after all.
But anyway. If you still believe this code should not be moved, please, confirm, I'll suggest the next patch version without it.
Right, I don't want to move it.
Vladis, if you take someone's patch as the base, you have to carry the original authorship somewhere...
Yes, I was thinking about it, I was just not sure how should I do it. Will the following form be fine? Or somehow else?
Based on a patch by Takashi Iwai" tiwai@suse.de
Yes, usually such a line should be enough.
- if not, create a new pcm stream. the caller must remove fp from
- the substream fmt_list in the error path to avoid double-free.
This comment isn't true. The caller may leave it as is, too, like my first version. It just depends on the code.
Yes. Is the following rewrite acceptable for the next patch version?
- if not, create a new pcm stream. Note, fp is added to the substream fmt_list
- and will be freed on the chip instance release. Do not free fp or do remove
- it from the substream fmt_list to avoid double-free.
Yes, that looks more correct.
thanks,
Takashi