On March 11, 2019 10:55:57 AM PDT, "Yordan Karadzhov (VMware)" <y.karadz@xxxxxxxxx> wrote:
>
>
>On 11.03.19 г. 15:44 ч., Slavomir Kaslev wrote:
>> On Mon, Mar 11, 2019 at 3:20 PM Yordan Karadzhov (VMware)
>> <y.karadz@xxxxxxxxx> wrote:
>>>
>>>
>>>
>>> On 11.03.19 г. 14:09 ч., Slavomir Kaslev wrote:
>>>> On Thu, Mar 7, 2019 at 5:44 PM Yordan Karadzhov
><ykaradzhov@xxxxxxxxxx> wrote:
>>>>>
>>>>> const char *lib = plugin.toStdString().c_str();
>>>>>
>>>>> This line is a bad idea because the returned array may (will) be
>>>>> invalidated when the destructor of std::string is called.
>>>>>
>>>>> Signed-off-by: Yordan Karadzhov <ykaradzhov@xxxxxxxxxx>
>>>>> ---
>>>>> kernel-shark/src/KsUtils.cpp | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/kernel-shark/src/KsUtils.cpp
>b/kernel-shark/src/KsUtils.cpp
>>>>> index 34b2e2d..d7b1753 100644
>>>>> --- a/kernel-shark/src/KsUtils.cpp
>>>>> +++ b/kernel-shark/src/KsUtils.cpp
>>>>> @@ -439,7 +439,7 @@ void
>KsPluginManager::registerFromList(kshark_context *kshark_ctx)
>>>>>
>>>>> auto lamRegUser = [&kshark_ctx](const QString &plugin)
>>>>> {
>>>>> - const char *lib = plugin.toStdString().c_str();
>>>>> + const char *lib = plugin.toLocal8Bit().data();
>>>>> kshark_register_plugin(kshark_ctx, lib);
>>>>> };
>>>>>
>>>>> @@ -474,7 +474,7 @@ void
>KsPluginManager::unregisterFromList(kshark_context *kshark_ctx)
>>>>>
>>>>> auto lamUregUser = [&kshark_ctx](const QString &plugin)
>>>>> {
>>>>> - const char *lib = plugin.toStdString().c_str();
>>>>> + const char *lib = plugin.toLocal8Bit().data();
>>>>> kshark_unregister_plugin(kshark_ctx, lib);
>>>>> };
>>>>
>>>> Doesn't the new version have the same problem with the temporary
>QByteArray?
>>>>
>>>> How about saving the data in a local std::string/QByteArray
>variable?
>>>>
>>>> std::string lib = plugin.toStdString();
>>>> kshark_register_plugin(kshark_ctx, lib.c_str());
>>>>
>>>
>>> Hi Slavi,
>>>
>>> As far I can understand toStdString() will create a fresh
>std::string
>>> and this string will has its own copy of the characters. However,
>this
>>> copy gets out-of-scope as soon as we hit the semicolon of the line.
>>>
>>> My understanding was that toLocal8Bit().data() makes no copy so the
>>> array will go out-of-scope only when the QString is destroyed.
>>
>> I did some digging into QString's implementation. From my reading of
>> the code, toLocal8Bit() calls into qt_convert_to_latin1()[1] which
>> does allocation, copying/transform-to-latin1 and returns the
>> QByteArray. So it seems that toLocal8Bit() is still making a copy.
>>
>> [1]
>https://github.com/qt/qtbase/blob/5.12/src/corelib/tools/qstring.cpp#L5275
>>
>> Cheers,
>>
>
>Thanks a lot for investigating this!
>I think in this case it will be more appropriate if you sign the patch.
>cheers,
>Y.
>
>> -- Slavi
>>
>>>
>>> But anyway, your solution looks cleaner and safer.
>>> I will send another version of the patch.
>>> Thanks a lot!
Signing the patch means that the person either wrote the patch or handled it to be committed.
I think you want Suggested-by:
-- Steve
>>> Y.
>>>
>>>
>>>> Thanks!
>>>>
>>>>
>>>> --
>>>> Slavomir Kaslev
>>>>
>>
>>
>>
--
Sent from my Android device with K-9 Mail. Please excuse my brevity and top posting.
|