* [PATCH v2 0/4] Various minor modifications and fixes toward KS 1.0
@ 2019-03-07 15:43 Yordan Karadzhov
2019-03-07 15:43 ` [PATCH v2 1/4] kernel-shark: Specify the OpenGL interface used by KernelShark Yordan Karadzhov
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Yordan Karadzhov @ 2019-03-07 15:43 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, Yordan Karadzhov
Please ignor the prevoius version of this patch set.
In particular patch [2/4] which has a sintax error.
I am sorry for the SPAM!
Yordan
Yordan Karadzhov (4):
kernel-shark: Specify the OpenGL interface used by KernelShark
kernel-shark: Fix a bug in ksmodel_zoom
kernel-shark: Fix Doxygen warning from sched_events
kernel-shark: Fix a bug in KsPluginManager
kernel-shark/CMakeLists.txt | 1 +
kernel-shark/src/KsUtils.cpp | 4 ++--
kernel-shark/src/libkshark-model.c | 4 ++--
kernel-shark/src/plugins/sched_events.c | 2 +-
4 files changed, 6 insertions(+), 5 deletions(-)
--
2.19.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/4] kernel-shark: Specify the OpenGL interface used by KernelShark
2019-03-07 15:43 [PATCH v2 0/4] Various minor modifications and fixes toward KS 1.0 Yordan Karadzhov
@ 2019-03-07 15:43 ` Yordan Karadzhov
2019-03-07 15:43 ` [PATCH v2 2/4] kernel-shark: Fix a bug in ksmodel_zoom Yordan Karadzhov
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Yordan Karadzhov @ 2019-03-07 15:43 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, Yordan Karadzhov
We instruct CMAKE to prefer using the legacy libGL library, if
available. Although this is the default choice, the latest versions
of CMAKE are printing a warning message in the case when no explicit
preference is provided.
Suggested-by: Slavomir Kaslev <kaslevs@vmware.com>
Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
kernel-shark/CMakeLists.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel-shark/CMakeLists.txt b/kernel-shark/CMakeLists.txt
index 9460026..20ced14 100644
--- a/kernel-shark/CMakeLists.txt
+++ b/kernel-shark/CMakeLists.txt
@@ -22,6 +22,7 @@ include(${KS_DIR}/build/FindJSONC.cmake)
find_package(Doxygen)
+set(OpenGL_GL_PREFERENCE LEGACY)
find_package(OpenGL)
find_package(GLUT)
--
2.19.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/4] kernel-shark: Fix a bug in ksmodel_zoom
2019-03-07 15:43 [PATCH v2 0/4] Various minor modifications and fixes toward KS 1.0 Yordan Karadzhov
2019-03-07 15:43 ` [PATCH v2 1/4] kernel-shark: Specify the OpenGL interface used by KernelShark Yordan Karadzhov
@ 2019-03-07 15:43 ` Yordan Karadzhov
2019-03-13 13:51 ` Steven Rostedt
2019-03-07 15:43 ` [PATCH v2 3/4] kernel-shark: Fix Doxygen warning from sched_events Yordan Karadzhov
2019-03-07 15:43 ` [PATCH v2 4/4] kernel-shark: Fix a bug in KsPluginManager Yordan Karadzhov
3 siblings, 1 reply; 14+ messages in thread
From: Yordan Karadzhov @ 2019-03-07 15:43 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, Yordan Karadzhov
When zooming-in the model is supposed to avoid over-zooming by
recalculation the Scale factor. The new value of the Scale factor is
supposed to be such that the size of the bin cannot be smaller than 5
ns. This patch fixes a naive bug in the way the new scale value is
calculated. The bug was introduced in
f97e31f00 ("kernel-shark-qt: Introduce the visualization model used by the Qt-based KS")
but had no effect until
94efea960 ("kernel-shark-qt: Handle the case when the range of the model is too small")
because the ridiculous value of the Scale factor resulted in a very
small model range and because of this the modification of the model
was always rejected.
Fixes: f97e31f00 ("kernel-shark-qt: Introduce the visualization model used by the Qt-based KS")
Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
kernel-shark/src/libkshark-model.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel-shark/src/libkshark-model.c b/kernel-shark/src/libkshark-model.c
index 4bd1e2c..af3440b 100644
--- a/kernel-shark/src/libkshark-model.c
+++ b/kernel-shark/src/libkshark-model.c
@@ -648,8 +648,8 @@ static void ksmodel_zoom(struct kshark_trace_histo *histo,
* Avoid overzooming. If needed, adjust the Scale factor to a the value
* which provides bin_size >= 5.
*/
- if (zoom_in && range * (1 - r) < histo->n_bins * 5)
- r = 1 - (histo->n_bins * 5) / range;
+ if (zoom_in && (int) range * (1. - r) < histo->n_bins * 5)
+ r = 1. - (histo->n_bins * 5.) / range;
/*
* Now calculate the new range of the histo. Use the bin of the marker
--
2.19.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/4] kernel-shark: Fix Doxygen warning from sched_events
2019-03-07 15:43 [PATCH v2 0/4] Various minor modifications and fixes toward KS 1.0 Yordan Karadzhov
2019-03-07 15:43 ` [PATCH v2 1/4] kernel-shark: Specify the OpenGL interface used by KernelShark Yordan Karadzhov
2019-03-07 15:43 ` [PATCH v2 2/4] kernel-shark: Fix a bug in ksmodel_zoom Yordan Karadzhov
@ 2019-03-07 15:43 ` Yordan Karadzhov
2019-03-07 15:43 ` [PATCH v2 4/4] kernel-shark: Fix a bug in KsPluginManager Yordan Karadzhov
3 siblings, 0 replies; 14+ messages in thread
From: Yordan Karadzhov @ 2019-03-07 15:43 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, Yordan Karadzhov
There is no reason for find_wakeup_pid being non-static. In the same
time, because it is non-static, Doxygen complains about missing
documentation for this function.
Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
kernel-shark/src/plugins/sched_events.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel-shark/src/plugins/sched_events.c b/kernel-shark/src/plugins/sched_events.c
index c52fb29..724aa19 100644
--- a/kernel-shark/src/plugins/sched_events.c
+++ b/kernel-shark/src/plugins/sched_events.c
@@ -148,7 +148,7 @@ static void plugin_register_command(struct kshark_context *kshark_ctx,
tep_register_comm(kshark_ctx->pevent, comm, pid);
}
-int find_wakeup_pid(struct kshark_context *kshark_ctx, struct kshark_entry *e,
+static int find_wakeup_pid(struct kshark_context *kshark_ctx, struct kshark_entry *e,
struct tep_event *wakeup_event, struct tep_format_field *pid_field)
{
struct tep_record *record;
--
2.19.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 4/4] kernel-shark: Fix a bug in KsPluginManager
2019-03-07 15:43 [PATCH v2 0/4] Various minor modifications and fixes toward KS 1.0 Yordan Karadzhov
` (2 preceding siblings ...)
2019-03-07 15:43 ` [PATCH v2 3/4] kernel-shark: Fix Doxygen warning from sched_events Yordan Karadzhov
@ 2019-03-07 15:43 ` Yordan Karadzhov
2019-03-11 12:09 ` Slavomir Kaslev
3 siblings, 1 reply; 14+ messages in thread
From: Yordan Karadzhov @ 2019-03-07 15:43 UTC (permalink / raw)
To: rostedt; +Cc: linux-trace-devel, Yordan Karadzhov
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@vmware.com>
---
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);
};
--
2.19.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] kernel-shark: Fix a bug in KsPluginManager
2019-03-07 15:43 ` [PATCH v2 4/4] kernel-shark: Fix a bug in KsPluginManager Yordan Karadzhov
@ 2019-03-11 12:09 ` Slavomir Kaslev
2019-03-11 13:20 ` Yordan Karadzhov (VMware)
0 siblings, 1 reply; 14+ messages in thread
From: Slavomir Kaslev @ 2019-03-11 12:09 UTC (permalink / raw)
To: Yordan Karadzhov; +Cc: Steven Rostedt, linux-trace-devel
On Thu, Mar 7, 2019 at 5:44 PM Yordan Karadzhov <ykaradzhov@vmware.com> 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@vmware.com>
> ---
> 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());
Thanks!
--
Slavomir Kaslev
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] kernel-shark: Fix a bug in KsPluginManager
2019-03-11 12:09 ` Slavomir Kaslev
@ 2019-03-11 13:20 ` Yordan Karadzhov (VMware)
2019-03-11 13:44 ` Slavomir Kaslev
0 siblings, 1 reply; 14+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-03-11 13:20 UTC (permalink / raw)
To: Slavomir Kaslev, Yordan Karadzhov; +Cc: Steven Rostedt, linux-trace-devel
On 11.03.19 г. 14:09 ч., Slavomir Kaslev wrote:
> On Thu, Mar 7, 2019 at 5:44 PM Yordan Karadzhov <ykaradzhov@vmware.com> 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@vmware.com>
>> ---
>> 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.
But anyway, your solution looks cleaner and safer.
I will send another version of the patch.
Thanks a lot!
Y.
> Thanks!
>
>
> --
> Slavomir Kaslev
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] kernel-shark: Fix a bug in KsPluginManager
2019-03-11 13:20 ` Yordan Karadzhov (VMware)
@ 2019-03-11 13:44 ` Slavomir Kaslev
2019-03-11 17:55 ` Yordan Karadzhov (VMware)
0 siblings, 1 reply; 14+ messages in thread
From: Slavomir Kaslev @ 2019-03-11 13:44 UTC (permalink / raw)
To: Yordan Karadzhov (VMware)
Cc: Yordan Karadzhov, Steven Rostedt, linux-trace-devel
On Mon, Mar 11, 2019 at 3:20 PM Yordan Karadzhov (VMware)
<y.karadz@gmail.com> wrote:
>
>
>
> On 11.03.19 г. 14:09 ч., Slavomir Kaslev wrote:
> > On Thu, Mar 7, 2019 at 5:44 PM Yordan Karadzhov <ykaradzhov@vmware.com> 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@vmware.com>
> >> ---
> >> 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,
-- Slavi
>
> But anyway, your solution looks cleaner and safer.
> I will send another version of the patch.
> Thanks a lot!
> Y.
>
>
> > Thanks!
> >
> >
> > --
> > Slavomir Kaslev
> >
--
Slavomir Kaslev
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] kernel-shark: Fix a bug in KsPluginManager
2019-03-11 13:44 ` Slavomir Kaslev
@ 2019-03-11 17:55 ` Yordan Karadzhov (VMware)
2019-03-11 18:15 ` Steven Rostedt
0 siblings, 1 reply; 14+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-03-11 17:55 UTC (permalink / raw)
To: Slavomir Kaslev; +Cc: Yordan Karadzhov, Steven Rostedt, linux-trace-devel
On 11.03.19 г. 15:44 ч., Slavomir Kaslev wrote:
> On Mon, Mar 11, 2019 at 3:20 PM Yordan Karadzhov (VMware)
> <y.karadz@gmail.com> wrote:
>>
>>
>>
>> On 11.03.19 г. 14:09 ч., Slavomir Kaslev wrote:
>>> On Thu, Mar 7, 2019 at 5:44 PM Yordan Karadzhov <ykaradzhov@vmware.com> 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@vmware.com>
>>>> ---
>>>> 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!
>> Y.
>>
>>
>>> Thanks!
>>>
>>>
>>> --
>>> Slavomir Kaslev
>>>
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] kernel-shark: Fix a bug in KsPluginManager
2019-03-11 17:55 ` Yordan Karadzhov (VMware)
@ 2019-03-11 18:15 ` Steven Rostedt
2019-03-26 1:58 ` Steven Rostedt
0 siblings, 1 reply; 14+ messages in thread
From: Steven Rostedt @ 2019-03-11 18:15 UTC (permalink / raw)
To: Yordan Karadzhov (VMware), Slavomir Kaslev
Cc: Yordan Karadzhov, linux-trace-devel
On March 11, 2019 10:55:57 AM PDT, "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
>
>
>On 11.03.19 г. 15:44 ч., Slavomir Kaslev wrote:
>> On Mon, Mar 11, 2019 at 3:20 PM Yordan Karadzhov (VMware)
>> <y.karadz@gmail.com> wrote:
>>>
>>>
>>>
>>> On 11.03.19 г. 14:09 ч., Slavomir Kaslev wrote:
>>>> On Thu, Mar 7, 2019 at 5:44 PM Yordan Karadzhov
><ykaradzhov@vmware.com> 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@vmware.com>
>>>>> ---
>>>>> 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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/4] kernel-shark: Fix a bug in ksmodel_zoom
2019-03-07 15:43 ` [PATCH v2 2/4] kernel-shark: Fix a bug in ksmodel_zoom Yordan Karadzhov
@ 2019-03-13 13:51 ` Steven Rostedt
2019-03-13 13:57 ` Yordan Karadzhov
0 siblings, 1 reply; 14+ messages in thread
From: Steven Rostedt @ 2019-03-13 13:51 UTC (permalink / raw)
To: Yordan Karadzhov; +Cc: linux-trace-devel
On Thu, 7 Mar 2019 17:43:14 +0200
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
> When zooming-in the model is supposed to avoid over-zooming by
> recalculation the Scale factor. The new value of the Scale factor is
> supposed to be such that the size of the bin cannot be smaller than 5
> ns. This patch fixes a naive bug in the way the new scale value is
> calculated.
BTW, what if the resolution of time stamps isn't in ns, and we can have
more than one event in a 5 unit bin. Does that mean we never display
multiple events in that small of a time scale?
-- Steve
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/4] kernel-shark: Fix a bug in ksmodel_zoom
2019-03-13 13:51 ` Steven Rostedt
@ 2019-03-13 13:57 ` Yordan Karadzhov
0 siblings, 0 replies; 14+ messages in thread
From: Yordan Karadzhov @ 2019-03-13 13:57 UTC (permalink / raw)
To: Steven Rostedt; +Cc: linux-trace-devel
On 13.03.19 г. 15:51 ч., Steven Rostedt wrote:
> On Thu, 7 Mar 2019 17:43:14 +0200
> Yordan Karadzhov <ykaradzhov@vmware.com> wrote:
>
>> When zooming-in the model is supposed to avoid over-zooming by
>> recalculation the Scale factor. The new value of the Scale factor is
>> supposed to be such that the size of the bin cannot be smaller than 5
>> ns. This patch fixes a naive bug in the way the new scale value is
>> calculated.
>
> BTW, what if the resolution of time stamps isn't in ns, and we can have
> more than one event in a 5 unit bin. Does that mean we never display
> multiple events in that small of a time scale?
For the moment Yes.
We have to make this value (5 units /ns) a parameter that can be configured.
Yordan
>
> -- Steve
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] kernel-shark: Fix a bug in KsPluginManager
2019-03-11 18:15 ` Steven Rostedt
@ 2019-03-26 1:58 ` Steven Rostedt
2019-03-26 13:37 ` Yordan Karadzhov (VMware)
0 siblings, 1 reply; 14+ messages in thread
From: Steven Rostedt @ 2019-03-26 1:58 UTC (permalink / raw)
To: Yordan Karadzhov (VMware), Slavomir Kaslev
Cc: Yordan Karadzhov, linux-trace-devel
On Mon, 11 Mar 2019 11:15:40 -0700
Steven Rostedt <rostedt@goodmis.org> wrote:
> >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:
>
BTW, I'm confused. Does this patch need to be applied or was there another version coming?
-- Steve
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] kernel-shark: Fix a bug in KsPluginManager
2019-03-26 1:58 ` Steven Rostedt
@ 2019-03-26 13:37 ` Yordan Karadzhov (VMware)
0 siblings, 0 replies; 14+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-03-26 13:37 UTC (permalink / raw)
To: Steven Rostedt, Slavomir Kaslev; +Cc: Yordan Karadzhov, linux-trace-devel
On 26.03.19 г. 3:58 ч., Steven Rostedt wrote:
> On Mon, 11 Mar 2019 11:15:40 -0700
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
>>> 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:
>>
>
> BTW, I'm confused. Does this patch need to be applied or was there another version coming?
>
It is my fault. Looks like I forgot to send v3.
The patch is coming.
Thanks!
Y.
> -- Steve
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-03-26 13:37 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-07 15:43 [PATCH v2 0/4] Various minor modifications and fixes toward KS 1.0 Yordan Karadzhov
2019-03-07 15:43 ` [PATCH v2 1/4] kernel-shark: Specify the OpenGL interface used by KernelShark Yordan Karadzhov
2019-03-07 15:43 ` [PATCH v2 2/4] kernel-shark: Fix a bug in ksmodel_zoom Yordan Karadzhov
2019-03-13 13:51 ` Steven Rostedt
2019-03-13 13:57 ` Yordan Karadzhov
2019-03-07 15:43 ` [PATCH v2 3/4] kernel-shark: Fix Doxygen warning from sched_events Yordan Karadzhov
2019-03-07 15:43 ` [PATCH v2 4/4] kernel-shark: Fix a bug in KsPluginManager Yordan Karadzhov
2019-03-11 12:09 ` Slavomir Kaslev
2019-03-11 13:20 ` Yordan Karadzhov (VMware)
2019-03-11 13:44 ` Slavomir Kaslev
2019-03-11 17:55 ` Yordan Karadzhov (VMware)
2019-03-11 18:15 ` Steven Rostedt
2019-03-26 1:58 ` Steven Rostedt
2019-03-26 13:37 ` Yordan Karadzhov (VMware)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).