[vlc-devel] [PATCH 3/3] Initialize title strings with non NULL pointer

Daniel Glaas daniel.glaas at freenet.de
Tue Jul 21 01:36:28 CEST 2020


Hello Thomas,

thank you for your review and you feedback, you pointed on some very new 
aspects for me ...

I implemented the second alternative solution you proposed, by 
introducing a special static const string with is assigned during 
initialization of the variables and against which comparisons are done. 
The way I coded this in line 75f. is surely not the best was, after 
trying several minutes around, I was not able to find a better solution 
that doesn't rise compiler warnings. I appreciate your suggestions.

And of course you are right, the strdup("") calls lead to memory leaks 
as I never freed them. I'm sorry that I didn't notice that, thanks for 
noticing this.

Am 20.07.20 um 08:33 schrieb Thomas Guillem:
> Hello,
>
> On Sat, Jul 18, 2020, at 12:47, Daniel Glaas wrote:
>> In principle, looking over all possible use cases of strdup I agree that
>> there might be case where strdup() returns NULL. But here I used it with
>> an empty string as parameter, meaning that malloc() only needs to
>> allocate one single byte in the memory. If this really fails, then there
>> are other problems than only a wrong display of an RSS text overlay.
> We require all allocations to be checked and handled. This is not the 
> case for every C programs and libs, but it is in VLC.
>
>
>> I also tried to add catches to the NULL pointers at those places where
>> they are used. But in my opinion, this makes the places of usage very
>> bad to read with a lot of NULL checks at various places. A lot of places
>> in the *Filter() function rely on valid pointers where the titles are
>> stored.
>>
> I see 2 other choices :
> - fix psz_title NULLITY check everywhere
> - Assign it to a special static const string (likely "") : in that 
> case, you should check that the pointer is not equals to this special 
> string pointer before freeing it.
>
> Also, your patch seems to leak the "" allocation when psz_title is 
> replaced by a new string.
>
>> Am 18.07.20 um 09:41 schrieb Rémi Denis-Courmont:
>>> Le lauantaina 18. heinäkuuta 2020, 5.07.00 EEST Daniel Glaas a écrit :
>>>> Initialize title strings with non NULL pointer
>>>>
>>>> If no valid title could be parsed out of an RSS feed, the 
>>>> segmentation fault
>>>> caused VLC to crash. The reason therefore was that the expected 
>>>> pointers to
>>>> the char buffers containing the titles were NULL pointers.
>>> This won't work because strdup() can return NULL. Better handle the NULL
>>> values correctly.
>>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Initialize-title-strings-with-static-const-empty-cha.patch
Type: text/x-patch
Size: 4679 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200721/145b1110/attachment.bin>


More information about the vlc-devel mailing list