On Wed, Apr 05, 2023 at 04:44:10PM +0100, Bruce Richardson wrote: > On Alpine linux, the telemetry_data_autotest was failing for the > test where we had dictionaries embedded in other dictionaries up > to three levels deep. Indications are that this issue is due to > excess data being stored on the stack, so replace stack-allocated > buffer data with dynamically allocated data in the case where we > are doing recursive processing of telemetry data structures into > json. > > Bugzilla ID: 1177 > Fixes: c933bb5177ca ("telemetry: support array values in data object") > Fixes: d2671e642a8e ("telemetry: support dict of dicts") > Cc: sta...@dpdk.org > > Signed-off-by: Bruce Richardson <bruce.richard...@intel.com> > > --- Acked-by: Tyler Retzlaff <roret...@linux.microsoft.com>
(one observation below) > V2: > set '\0' in newly malloc'ed buffer to ensure it always has valid > string data. > --- > lib/telemetry/telemetry.c | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c > index 7bceadcee7..728a0bffd4 100644 > --- a/lib/telemetry/telemetry.c > +++ b/lib/telemetry/telemetry.c > @@ -208,7 +208,11 @@ container_to_json(const struct rte_tel_data *d, char > *out_buf, size_t buf_len) > break; > case RTE_TEL_CONTAINER: > { > - char temp[buf_len]; > + char *temp = malloc(buf_len); > + if (temp == NULL) > + break; > + *temp = '\0'; /* ensure valid string */ > + > const struct container *cont = > &v->value.container; > if (container_to_json(cont->data, > @@ -219,6 +223,7 @@ container_to_json(const struct rte_tel_data *d, char > *out_buf, size_t buf_len) > v->name, temp); > if (!cont->keep) > rte_tel_data_free(cont->data); > + free(temp); > break; > } > } > @@ -275,7 +280,11 @@ output_json(const char *cmd, const struct rte_tel_data > *d, int s) > break; > case RTE_TEL_CONTAINER: > { > - char temp[buf_len]; > + char *temp = malloc(buf_len); > + if (temp == NULL) > + break; > + *temp = '\0'; /* ensure valid string */ > + > const struct container *cont = > &v->value.container; > if (container_to_json(cont->data, > @@ -286,6 +295,7 @@ output_json(const char *cmd, const struct rte_tel_data > *d, int s) > v->name, temp); > if (!cont->keep) > rte_tel_data_free(cont->data); > + free(temp); not expressing a preference just noticing that when RTE_TEL_CONTAINER cases are the last case in the switch sometimes there is an explicit break; and sometimes not. > } > } > } > @@ -311,7 +321,11 @@ output_json(const char *cmd, const struct rte_tel_data > *d, int s) > buf_len, used, > d->data.array[i].uval); > else if (d->type == TEL_ARRAY_CONTAINER) { > - char temp[buf_len]; > + char *temp = malloc(buf_len); > + if (temp == NULL) > + break; > + *temp = '\0'; /* ensure valid string */ > + > const struct container *rec_data = > &d->data.array[i].container; > if (container_to_json(rec_data->data, > @@ -321,6 +335,7 @@ output_json(const char *cmd, const struct rte_tel_data > *d, int s) > buf_len, used, temp); > if (!rec_data->keep) > rte_tel_data_free(rec_data->data); > + free(temp); > } > break; > } > -- > 2.37.2