[vlc-devel] [PATCH] vlc_es: add more documentation on es_format_Copy

[Steve Lhomme]

On Wed, Jul 12, 2017 at 5:24 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:
> On mercredi 12 juillet 2017 16:51:19 EEST Steve Lhomme wrote:
>> It must copy to an unitialized structure or one that was cleaned before.
>>
>> The result should always be checked as there are allocations that can fail
>> but it's not safe for now given the state of the rest of the code.
>> ---
>>  include/vlc_es.h | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/include/vlc_es.h b/include/vlc_es.h
>> index e9d11d85a8..a428b561c2 100644
>> --- a/include/vlc_es.h
>> +++ b/include/vlc_es.h
>> @@ -627,7 +627,10 @@ VLC_API void es_format_InitFromVideo( es_format_t *,
>> const video_format_t * );
>>
>>  /**
>>   * This functions will copy a es_format_t.
>> + *
>> + * The destination pointer must be uninitialized or not old any allocated

Typo "hold"

>> buffer. */
>> +/* VLC_USED for now we're okay with losing strings and extradata */
>>  VLC_API int es_format_Copy( es_format_t *p_dst, const es_format_t *p_src );
>>
>>  /**
>
> No. The error is impossible in some code paths (e.g. audio filters), and
> totally harmless in others. Littering the code base with untested, buggy and
> sometimes dead, error code does not make sense.
>
> You can't add VLC_USED then, since ignoring the result is not always a bug.

OK

> And video.p_palette should die. And language should be a small fixed size

I think the reason to use a union was to save space. If we force
psz_language and psz_description to fixed size buffers we lose that
advantage and we gain strings that may not always fit the strings we
want to store.

> character array. That leaves only p_extra IIRC.

There's also p_extra_languages, subs.psz_encoding and subs.p_style.

So dynamically allocated memory in es_format_t is not going away
anytime soon (if ever). We should make sure it's handled properly and
make it harder to have leaks.

> --
> Rémi Denis-Courmont
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel