| Message ID | 20180711133709.26788-1-y.karadz@gmail.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [1/2] kernel-shark-qt: Deal with the "no tasks" case in kshark_get_task_pids() | expand |
On Wed, 11 Jul 2018 16:37:08 +0300 "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote: > kshark_get_task_pids() should not return a "memory allocation" error > in the case when it is called before having trace data loaded. > > Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com> > --- > kernel-shark-qt/src/libkshark.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c > index 668b6df..75b88c9 100644 > --- a/kernel-shark-qt/src/libkshark.c > +++ b/kernel-shark-qt/src/libkshark.c > @@ -303,6 +303,12 @@ ssize_t kshark_get_task_pids(struct kshark_context *kshark_ctx, int **pids) > } > } > > + if (!pid_count) { > + free(*pids); > + *pids = NULL; > + return pid_count; > + } > + > temp_pids = realloc(*pids, pid_count * sizeof(int)); > if (!temp_pids) > goto fail; What about doing it this way: diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c index 668b6df8..34daa25e 100644 --- a/kernel-shark-qt/src/libkshark.c +++ b/kernel-shark-qt/src/libkshark.c @@ -303,12 +303,17 @@ ssize_t kshark_get_task_pids(struct kshark_context *kshark_ctx, int **pids) } } - temp_pids = realloc(*pids, pid_count * sizeof(int)); - if (!temp_pids) - goto fail; + if (pid_count) { + temp_pids = realloc(*pids, pid_count * sizeof(int)); + if (!temp_pids) + goto fail; - /* Paranoid: In the unlikely case of shrinking *pids, realloc moves it */ - *pids = temp_pids; + /* Paranoid: In the unlikely case of shrinking *pids, realloc moves it */ + *pids = temp_pids; + } else { + free(*pids); + *pids = NULL; + } return pid_count; ? -- Steve
On 11.07.2018 17:02, Steven Rostedt wrote: > On Wed, 11 Jul 2018 16:37:08 +0300 > "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote: > >> kshark_get_task_pids() should not return a "memory allocation" error >> in the case when it is called before having trace data loaded. >> >> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com> >> --- >> kernel-shark-qt/src/libkshark.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c >> index 668b6df..75b88c9 100644 >> --- a/kernel-shark-qt/src/libkshark.c >> +++ b/kernel-shark-qt/src/libkshark.c >> @@ -303,6 +303,12 @@ ssize_t kshark_get_task_pids(struct kshark_context *kshark_ctx, int **pids) >> } >> } >> >> + if (!pid_count) { >> + free(*pids); >> + *pids = NULL; >> + return pid_count; >> + } >> + >> temp_pids = realloc(*pids, pid_count * sizeof(int)); >> if (!temp_pids) >> goto fail; > > What about doing it this way: > > diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c > index 668b6df8..34daa25e 100644 > --- a/kernel-shark-qt/src/libkshark.c > +++ b/kernel-shark-qt/src/libkshark.c > @@ -303,12 +303,17 @@ ssize_t kshark_get_task_pids(struct kshark_context *kshark_ctx, int **pids) > } > } > > - temp_pids = realloc(*pids, pid_count * sizeof(int)); > - if (!temp_pids) > - goto fail; > + if (pid_count) { > + temp_pids = realloc(*pids, pid_count * sizeof(int)); > + if (!temp_pids) > + goto fail; > > - /* Paranoid: In the unlikely case of shrinking *pids, realloc moves it */ > - *pids = temp_pids; > + /* Paranoid: In the unlikely case of shrinking *pids, realloc moves it */ > + *pids = temp_pids; > + } else { > + free(*pids); > + *pids = NULL; > + } > > return pid_count; > Yes, this way is better. Thanks! Yordan > > ? > > -- Steve >
Hi Steven, Should I send this patch again? Thanks! Yordan On 11.07.2018 17:08, Yordan Karadzhov (VMware) wrote: >> >> What about doing it this way: >> >> diff --git a/kernel-shark-qt/src/libkshark.c >> b/kernel-shark-qt/src/libkshark.c >> index 668b6df8..34daa25e 100644 >> --- a/kernel-shark-qt/src/libkshark.c >> +++ b/kernel-shark-qt/src/libkshark.c >> @@ -303,12 +303,17 @@ ssize_t kshark_get_task_pids(struct >> kshark_context *kshark_ctx, int **pids) >> } >> } >> - temp_pids = realloc(*pids, pid_count * sizeof(int)); >> - if (!temp_pids) >> - goto fail; >> + if (pid_count) { >> + temp_pids = realloc(*pids, pid_count * sizeof(int)); >> + if (!temp_pids) >> + goto fail; >> - /* Paranoid: In the unlikely case of shrinking *pids, realloc >> moves it */ >> - *pids = temp_pids; >> + /* Paranoid: In the unlikely case of shrinking *pids, realloc >> moves it */ >> + *pids = temp_pids; >> + } else { >> + free(*pids); >> + *pids = NULL; >> + } >> return pid_count; > > Yes, this way is better. > > Thanks! > Yordan > > >> >> ? >> >> -- Steve
On Fri, 3 Aug 2018 16:06:24 +0300 "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote: > Hi Steven, > Should I send this patch again? Yes, with the updates. Thanks! -- Steve
On Fri, 3 Aug 2018 13:46:22 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Fri, 3 Aug 2018 16:06:24 +0300 > "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote: > > > Hi Steven, > > Should I send this patch again? > > Yes, with the updates. > I take this back. I'll do the updates and apply your other patches on top. Thanks! -- Steve
On Fri, 3 Aug 2018 14:18:27 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Fri, 3 Aug 2018 13:46:22 -0400 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > On Fri, 3 Aug 2018 16:06:24 +0300 > > "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote: > > > > > Hi Steven, > > > Should I send this patch again? > > > > Yes, with the updates. > > > > I take this back. I'll do the updates and apply your other patches on > top. > And this is what I get when I work on too many projects at once. I already have the update applied to my local tree :-p. -- Steve
diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c index 668b6df..75b88c9 100644 --- a/kernel-shark-qt/src/libkshark.c +++ b/kernel-shark-qt/src/libkshark.c @@ -303,6 +303,12 @@ ssize_t kshark_get_task_pids(struct kshark_context *kshark_ctx, int **pids) } } + if (!pid_count) { + free(*pids); + *pids = NULL; + return pid_count; + } + temp_pids = realloc(*pids, pid_count * sizeof(int)); if (!temp_pids) goto fail;
kshark_get_task_pids() should not return a "memory allocation" error in the case when it is called before having trace data loaded. Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com> --- kernel-shark-qt/src/libkshark.c | 6 ++++++ 1 file changed, 6 insertions(+)