On Mon, Dec 20, 2021 at 2:04 AM Andreas Rheinhardt <andreas.rheinha...@outlook.com> wrote: > > Diederick Niehorster: > > // store to device_list output > > if (av_reallocp_array(&(*device_list)->devices, > > (*device_list)->nb_devices + 1, > > @@ -412,6 +417,8 @@ dshow_cycle_devices(AVFormatContext *avctx, > > ICreateDevEnum *devenum, > > av_freep(&device->device_name); > > if (device->device_name) > > av_freep(&device->device_description); > > + if (device->media_types) > > + av_freep(&device->media_types); > > You are duplicating freeing code here: You have code to free media_types > both before it was put into device and after; you can avoid the latter > by only attaching media_types to device when nothing else can fail.
I could indeed avoid adding av_freep(&device->media_types) when only attaching media_types to the device once it made it into the device list array (so doing (*device_list)->devices[(*device_list)->nb_devices]->media_types = media_types). But that makes the code harder to read (asymmetric, everything else is attached to the device before it goes into the list) and the missing free while the other device members are freed looks like a code smell. I think this is easier to read and less error prone for future patches when done as currently. Thanks! Dee _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".