[vlc-devel] [PATCH 1/3] qt: use shared pointers in menu items

[Rémi Denis-Courmont]

I don't understand your points. Hold has to be there manually because that's just how the class work.

And I don't see what manual calls to release you are referring to. The trivial wrappers are needed for casting. This is a mismatch between C and C++, that I only hilights that C++ should be avoided in vlc.git.

Le 20 mars 2019 18:23:15 GMT+08:00, "Hugo Beauzée-Luyssen" <hugo at beauzee.fr> a écrit :
>On Mon, Mar 18, 2019, at 4:37 PM, Rémi Denis-Courmont wrote:
>> Le lundi 18 mars 2019, 15:20:14 EET Hugo Beauzée-Luyssen a écrit :
>> > Hi,
>> > 
>> > I like the idea but why using QSharedPointer instead of
>vlc_shared_data_ptr
>> > or wrap_cptr?
>> 
>> AFAIU, vlc_shared_data_ptr and the Qt equivalent are meant for, well,
>shared 
>> data. Also AFAIU, vlc_shared_data_ptr would not work here since it 
>> instantiates one type per pair of hold/release functions, and we need
>one 
>> single type for all four possible pairs (none, input, aout, vout).
>> 
>
>Fair point
>
>> As for wrap_cptr, it seems to me that Qt code uses Qt rather than VLC
>C++, or 
>> even ISO C++, helpers, e.g. QString rather than std::string and
>QMutexLocker 
>> rather than vlc_mutex_locker.
>> 
>
>QString is the only type we can use if the string needs to be
>displayed, so I'm not sure if the comparison is fair/relevant.
>Regarding mutexes, as far as I can see most locking is done manually,
>and the use of QMutexLocker/vlc_mutex_locker is marginal.
>
>That being said, what mostly concerns me is that even though a smart
>pointer is used, there are still manual calls to hold/release.
>
>
>-- 
>  Hugo Beauzée-Luyssen
>  hugo at beauzee.fr
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190321/c224de50/attachment.html>