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@xxxxxxx> 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 _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
