| Message ID | 20190314151012.905-12-ykaradzhov@vmware.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Various modifications and fixes toward KS 1.0 | expand |
On Thu, Mar 14, 2019 at 5:11 PM Yordan Karadzhov <ykaradzhov@vmware.com> wrote: > > ksmodel_set_bin_counts() should not crash in the case when all > bins of the model, except the Upper Overflow bin, are empty. > > Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com> > --- > kernel-shark/src/libkshark-model.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/kernel-shark/src/libkshark-model.c b/kernel-shark/src/libkshark-model.c > index af3440b..29676c7 100644 > --- a/kernel-shark/src/libkshark-model.c > +++ b/kernel-shark/src/libkshark-model.c > @@ -290,7 +290,7 @@ static void ksmodel_set_next_bin_edge(struct kshark_trace_histo *histo, > static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo) > { > int i = 0, prev_not_empty; > - ssize_t count_tmp; > + ssize_t count_tmp = 0; > > histo->tot_count = 0; > memset(&histo->bin_count[0], 0, > @@ -303,7 +303,7 @@ static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo) > prev_not_empty = LOB(histo); > } else { > /* Loop till the first non-empty bin. */ > - while (histo->map[i] < 0) { > + while (histo->map[i] < 0 && i < histo->n_bins) { I would suggest switching the order of those checks while (i < histo->n_bins && histo->map[i] < 0) { It's safe as is since hist->map has histo->n_bins+2 entries but checking bounds before dereferencing is more idiomatic. -- Slavi > ++i; > } > > @@ -316,7 +316,8 @@ static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo) > */ > for (; i < histo->n_bins; ++i) { > if (histo->map[i] != KS_EMPTY_BIN) { > - /* The current bin is not empty, take its data row and > + /* > + * The current bin is not empty, take its data row and > * subtract it from the data row of the previous not > * empty bin, which will give us the number of data > * rows in the "prev_not_empty" bin. > @@ -358,7 +359,7 @@ static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo) > * We will do a sanity check. The number of data rows in the last not > * empty bin must be greater than zero. > */ > - assert(count_tmp > 0); > + assert(count_tmp >= 0); > histo->tot_count += histo->bin_count[prev_not_empty] = count_tmp; > } > > -- > 2.19.1 >
diff --git a/kernel-shark/src/libkshark-model.c b/kernel-shark/src/libkshark-model.c index af3440b..29676c7 100644 --- a/kernel-shark/src/libkshark-model.c +++ b/kernel-shark/src/libkshark-model.c @@ -290,7 +290,7 @@ static void ksmodel_set_next_bin_edge(struct kshark_trace_histo *histo, static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo) { int i = 0, prev_not_empty; - ssize_t count_tmp; + ssize_t count_tmp = 0; histo->tot_count = 0; memset(&histo->bin_count[0], 0, @@ -303,7 +303,7 @@ static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo) prev_not_empty = LOB(histo); } else { /* Loop till the first non-empty bin. */ - while (histo->map[i] < 0) { + while (histo->map[i] < 0 && i < histo->n_bins) { ++i; } @@ -316,7 +316,8 @@ static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo) */ for (; i < histo->n_bins; ++i) { if (histo->map[i] != KS_EMPTY_BIN) { - /* The current bin is not empty, take its data row and + /* + * The current bin is not empty, take its data row and * subtract it from the data row of the previous not * empty bin, which will give us the number of data * rows in the "prev_not_empty" bin. @@ -358,7 +359,7 @@ static void ksmodel_set_bin_counts(struct kshark_trace_histo *histo) * We will do a sanity check. The number of data rows in the last not * empty bin must be greater than zero. */ - assert(count_tmp > 0); + assert(count_tmp >= 0); histo->tot_count += histo->bin_count[prev_not_empty] = count_tmp; }
ksmodel_set_bin_counts() should not crash in the case when all bins of the model, except the Upper Overflow bin, are empty. Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com> --- kernel-shark/src/libkshark-model.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)