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 83B9BA0093; Fri, 17 Jun 2022 13:16:11 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2664140DDD; Fri, 17 Jun 2022 13:16:11 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 9342440698 for ; Fri, 17 Jun 2022 13:16:09 +0200 (CEST) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH v2 1/5] telemetry: escape special char when tel string X-MimeOLE: Produced By Microsoft Exchange V6.5 Date: Fri, 17 Jun 2022 13:16:08 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D8713C@smartserver.smartshare.dk> In-Reply-To: <20220617094624.17578-2-fengchengwen@huawei.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v2 1/5] telemetry: escape special char when tel string Thread-Index: AdiCMAjZ9JKlmeMDRsWMDC7l7YnM9gABX6rQ References: <20220615073915.14041-1-fengchengwen@huawei.com> <20220617094624.17578-1-fengchengwen@huawei.com> <20220617094624.17578-2-fengchengwen@huawei.com> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Chengwen Feng" , , , , , , , , Cc: 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 > From: Chengwen Feng [mailto:fengchengwen@huawei.com] > Sent: Friday, 17 June 2022 11.46 >=20 > This patch supports escape special characters (including: \",\\,/,\b, > /f,/n,/r,/t) when telemetry string. > This patch is used to support telemetry xxx-dump commands which the > string may include special characters. >=20 > Signed-off-by: Chengwen Feng > --- > lib/telemetry/telemetry.c | 96 = +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 93 insertions(+), 3 deletions(-) >=20 > diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c > index c6fd03a5ab..0f762f633e 100644 > --- a/lib/telemetry/telemetry.c > +++ b/lib/telemetry/telemetry.c > @@ -215,6 +215,94 @@ container_to_json(const struct rte_tel_data *d, > char *out_buf, size_t buf_len) > return used; > } >=20 > +static bool > +json_is_special_char(char ch) > +{ > + static unsigned char is_spec[256] =3D { 0 }; > + static bool init_once; > + > + if (!init_once) { > + is_spec['\"'] =3D 1; > + is_spec['\\'] =3D 1; > + is_spec['/'] =3D 1; > + is_spec['\b'] =3D 1; > + is_spec['\f'] =3D 1; > + is_spec['\n'] =3D 1; > + is_spec['\r'] =3D 1; > + is_spec['\t'] =3D 1; > + init_once =3D true; > + } > + > + return (bool)is_spec[(unsigned char)ch]; > +} Here's a suggestion for simplifying the code: Remove json_is_special_char(), and update json_escape_special_char() and = json_format_string() as follows: > + > +static size_t > +json_escape_special_char(char *buf, const char ch) Consider making this function inline. > +{ > + size_t used =3D 0; > + > + switch (ch) { > + case '\"': > + buf[used++] =3D '\\'; > + buf[used++] =3D '\"'; > + break; > + case '\\': > + buf[used++] =3D '\\'; > + buf[used++] =3D '\\'; > + break; > + case '/': > + buf[used++] =3D '\\'; > + buf[used++] =3D '/'; > + break; > + case '\b': > + buf[used++] =3D '\\'; > + buf[used++] =3D 'b'; > + break; > + case '\f': > + buf[used++] =3D '\\'; > + buf[used++] =3D 'f'; > + break; > + case '\n': > + buf[used++] =3D '\\'; > + buf[used++] =3D 'n'; > + break; > + case '\r': > + buf[used++] =3D '\\'; > + buf[used++] =3D 'r'; > + break; > + case '\t': > + buf[used++] =3D '\\'; > + buf[used++] =3D 't'; > + break; > + default: Handle non-escaped characters in the default case here instead: + buf[used++] =3D ch; > + break; > + } > + > + return used; > +} > + > +static size_t > +json_format_string(char *buf, size_t len, const char *str) > +{ > + size_t used =3D 0; > + > + while (*str) { > + if (unlikely(len < used + 2)) { > + TMTY_LOG(WARNING, "Insufficient buffer when json > format string\n"); > + break; > + } > + -- replace: > + if (json_is_special_char(*str)) > + used +=3D json_escape_special_char(buf + used, *str); > + else > + buf[used++] =3D *str; > + > + str++; -- by: + used +=3D json_escape_special_char(buf + used, *str++); -- End of suggestion. Feel free to use it or not. :-) > + } > + > + return used; > +} > + > static void > output_json(const char *cmd, const struct rte_tel_data *d, int s) > { > @@ -232,9 +320,11 @@ output_json(const char *cmd, const struct > rte_tel_data *d, int s) > MAX_CMD_LEN, cmd ? cmd : "none"); > break; > case RTE_TEL_STRING: > - used =3D snprintf(out_buf, sizeof(out_buf), > "{\"%.*s\":\"%.*s\"}", > - MAX_CMD_LEN, cmd, > - RTE_TEL_MAX_SINGLE_STRING_LEN, d->data.str); > + used =3D snprintf(out_buf, sizeof(out_buf), "{\"%.*s\":\"", > + MAX_CMD_LEN, cmd); > + used +=3D json_format_string(out_buf + used, > + sizeof(out_buf) - used - 3, d->data.str); > + used +=3D snprintf(out_buf + used, sizeof(out_buf) - used, > "\"}"); I looked for missing 0-termination in json_format_string(), but that is = not required due to the immediately following snprintf(). > break; > case RTE_TEL_DICT: > prefix_used =3D snprintf(out_buf, sizeof(out_buf), > "{\"%.*s\":", > -- > 2.33.0 >=20 If you want to make it generic, these four cases in output_json() also = need to JSON encode the strings: case RTE_TEL_DICT: case RTE_TEL_STRING_VAL: ... case RTE_TEL_CONTAINER: ... (strings only) case RTE_TEL_ARRAY_STRING: if (d->type =3D=3D RTE_TEL_ARRAY_STRING) ... else if (d->type =3D=3D RTE_TEL_ARRAY_CONTAINER) ... (strings only) However, JSON encoding of strings inside arrays and containers is not = required for the dump purposes addressed by this patch series, so I = consider this patch complete without it. No need to add "feature creep" = to this series. Reviewed-by: Morten Br=F8rup