[vlc-devel] [vlc-commits] demux: adaptive: missing es_format_Init

[Rémi Denis-Courmont]

Le 29 août 2017 14:59:00 GMT+03:00, Francois Cartegnie <fcvlcdev at free.fr> a écrit :
>Le 29/08/2017 à 13:26, Steve Lhomme a écrit :
>> On Tue, Aug 29, 2017 at 11:59 AM, Francois Cartegnie
><fcvlcdev at free.fr> wrote:
>>> Le 29/08/2017 à 11:51, Steve Lhomme a écrit :
>>>
>>>>>  FakeESOutID * FakeESOut::createNewID( const es_format_t *p_fmt )
>>>>>  {
>>>>>      es_format_t fmtcopy;
>>>>> +    es_format_Init( &fmtcopy, p_fmt->i_cat, p_fmt->i_codec );
>>>>
>>>> This is unneeded and has no effect. Copy() overwrites an
>uninitialized
>>>> es_format. If it's initialized it might actually leak dynamic that
>are
>>>> stored in it.
>>>>
>>>
>>> I give up explaining again why Copy shouldn't be used for init.
>> 
>> This is just what the code does and assumes everywhere. It's just
>doing a = b;
>> If you want a to be initialized you can call Clean() in Copy(). But
>> you'll have some weird behavior because in many cases that memory is
>> actually uninitialized and you'll end up freeing memory pointing to
>> nowhere.
>That was exactly the reason of enforcing Init() prior copy() everywhere
>so we could get finally rid of Init() done inside Copy() and prevent
>those undefined state cases.
>
>And since the ES changes, a = b isn't true, due to type specific
>pointers. We're just recreating leaks or shared pointer because we
>couldn't fix the api doing proper init use clean cycles.
>
>Francois Cartegnie
>VideoLAN - VLC Developer
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

Both patterns commonly exist in the wild: copy constructor (or POD assignment operator), vs overloaded assignment operator. Also neither patterns are fool-proof in C, since one can leak and the other can trigger UB.

However, the latter is completely useless and wasteful. It just performs a throw away initialization. It is also much rarer in pure C. Also leaking is preferable over UB if it comes to that. Lastly, it requires more code in C since we can't have implicit init.

I agree with Steve, and I already mentioned it before. I don't see any benefits to forcing init before copy.

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.