From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0505C428E8; Fri, 7 Apr 2023 21:21:19 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D066A40E03; Fri, 7 Apr 2023 21:21:18 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 28B8840150; Fri, 7 Apr 2023 21:21:17 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1086) id 70EBD2121EA8; Fri, 7 Apr 2023 12:21:16 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 70EBD2121EA8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1680895276; bh=dakUFAAzAuP2v8ajhAktKyfpMbaQ8TW8PsJPbwITC6A=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ihBDnRrq77qfhsARm8niZIsbDNhar30XlKyN2dRWjzCHDFsTjezGMhFTh/Fr43rJk +jKaFAQLPJpYMrIEUuV/K6VmNnU6eV3Dgz2ZHMj6bUrCGzthOKEG/5jc1HLAJSm6gP uXsghSD4MbNyFb+EbLd6gNk+mEPD6UhtnFFks+1M= Date: Fri, 7 Apr 2023 12:21:16 -0700 From: Tyler Retzlaff To: Bruce Richardson Cc: dev@dpdk.org, ciara.power@intel.com, stable@dpdk.org Subject: Re: [PATCH v2 1/5] telemetry: fix autotest failures on Alpine Message-ID: <20230407192116.GA3014@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20230310181836.162336-1-bruce.richardson@intel.com> <20230405154414.183915-1-bruce.richardson@intel.com> <20230405154414.183915-2-bruce.richardson@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230405154414.183915-2-bruce.richardson@intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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: stable@dpdk.org > > Signed-off-by: Bruce Richardson > > --- Acked-by: Tyler Retzlaff (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