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 631D9A09E7; Wed, 19 Oct 2022 18:48:14 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4592D42BF9; Wed, 19 Oct 2022 18:48:14 +0200 (CEST) Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id 8D9DE4280C for ; Wed, 19 Oct 2022 18:48:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1666198092; x=1697734092; h=date:from:to:cc:subject:message-id:references: in-reply-to:mime-version; bh=oYGoDotO+wpPoMkgK/iPrh3UioRe9NSMyoXqYRzE9yM=; b=GUgAsBIZS3DiZZZ9xLT/iw2p13wFbsYG9Rg2mz5RD7EBlyuYcasbE59q oFmUG/v/gqjL03Wa89a4Ke5dfVUaRVWHsQPjL/W+JxuT2EsqqsZw231/Z V24/FzZGG7uSlgP4zwdz+tYKCIRhuNFJjwsc7O+nB1R5emmPWn6IuxSF/ NOEHqhsuaNV62vCKFQlpZJW+DuseByZwrQdC8n2Om0h0kXsQFUAz4NsLD 3HBgr+lQ3jpLiHdXY3tV0xtqEB1XKPsMV57s9CSolWhoppCvoM/fZY0vj MwNvgem/rJ7Ae7bq/DRZxEZrBeseKTHu8PeBsHr9F1ofDH/Ceni9naDwi A==; X-IronPort-AV: E=McAfee;i="6500,9779,10505"; a="368521506" X-IronPort-AV: E=Sophos;i="5.95,196,1661842800"; d="scan'208";a="368521506" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Oct 2022 09:48:11 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10505"; a="734310779" X-IronPort-AV: E=Sophos;i="5.95,196,1661842800"; d="scan'208";a="734310779" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by fmsmga002.fm.intel.com with ESMTP; 19 Oct 2022 09:48:10 -0700 Received: from orsmsx612.amr.corp.intel.com (10.22.229.25) by ORSMSX601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Wed, 19 Oct 2022 09:48:10 -0700 Received: from orsmsx603.amr.corp.intel.com (10.22.229.16) by ORSMSX612.amr.corp.intel.com (10.22.229.25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Wed, 19 Oct 2022 09:48:09 -0700 Received: from ORSEDG601.ED.cps.intel.com (10.7.248.6) by orsmsx603.amr.corp.intel.com (10.22.229.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31 via Frontend Transport; Wed, 19 Oct 2022 09:48:09 -0700 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (104.47.55.103) by edgegateway.intel.com (134.134.137.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2375.31; Wed, 19 Oct 2022 09:48:08 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=RJ2NF14RekKlzkyEBgRgT0RX/ADDnQuFbNI02fNnstxakFsT+054hNjlOEa6fMPhwXzQEJeMEiVxM6TTlqLu+3RTcGvm+AYhVSWp77+1N6Z1L4WG77m8jJUZzFaktaqx5XaHdbb/Atxavru8QnSFrPyu1lvBCNGFFm5O0xoRgQ0VijCRN1ijMLEvn7zElouwYgZENO0HPBhorN7ix6l417RHucu62WK6ma3OeN/NUND7DapM7MjAR7dt/vwCWjqBGK9gQNwiG+TCcYx6D3+kc1VfscLG+OQApXT6g+bZb0FRwlZo6KMjBpo0eIEMYe+zlOk5TyY8i5WjxcV4BaetMg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=dHdf4HxDGrZhUT6yu/BRoufrleTuKRhk+hIIUm24pzc=; b=J6Rm/yD0oJEu6VUkZC286FYMRPV/Ojr+dUlJi2Q29sO6IZLIs9schODUXCceB0U/q3/kZ6lLxdjnuprRSP8mvY4PUXLy9gMiaIL2WI79UB8aBv/GPeKwuUybC1uglJ9XgUSCzlINYnsTcqb05yGKQqbqV7fdKxEOZW2e+P5EWA2/U7V6NqoErYLzC/HZAnFS7yZsRKcpB6i7mFG263+eah5B2VFNn9Af/nNjoG2DI02Qk5XBLxZkIXpsPz3Ko4f/P4TgqrOb2F2mSk8i1tHa8KQCD+FGZOFh/j7akPPTkKnR7b5B/sWpaWKCkRibMsvyITmmiP4X2ICk9qMvKBClPw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; Received: from MWHPR11MB1629.namprd11.prod.outlook.com (2603:10b6:301:d::21) by PH8PR11MB7094.namprd11.prod.outlook.com (2603:10b6:510:216::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5723.33; Wed, 19 Oct 2022 16:47:58 +0000 Received: from MWHPR11MB1629.namprd11.prod.outlook.com ([fe80::5582:9796:3aaa:aa1]) by MWHPR11MB1629.namprd11.prod.outlook.com ([fe80::5582:9796:3aaa:aa1%12]) with mapi id 15.20.5723.034; Wed, 19 Oct 2022 16:47:58 +0000 Date: Wed, 19 Oct 2022 17:47:51 +0100 From: Bruce Richardson To: David Marchand CC: , , Ciara Power Subject: Re: [PATCH] telemetry: support boolean type Message-ID: References: <20221019073702.3948624-1-david.marchand@redhat.com> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: X-ClientProxiedBy: LO4P123CA0475.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:1a8::12) To MWHPR11MB1629.namprd11.prod.outlook.com (2603:10b6:301:d::21) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MWHPR11MB1629:EE_|PH8PR11MB7094:EE_ X-MS-Office365-Filtering-Correlation-Id: 9f9cbc56-fa81-4f5f-1415-08dab1f1ad46 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 39VZjvFee/dXKpxl9ONIo/otQj1HkAViIRY6BlK9pyb9RheXBaLO9aoOXFVA/RV6s3uPT/78HNmXmr7pvuCdxzYEwpcvoDRLs9TU5fqA5wvTTY2OxBqOggUkLDlWuRL/+Rj0tzIFZX1XAUDd0XVuq+PlYnKtsOmrEewHyQJPSFiJy6+gmT+kMnwrgq55BUIgObfGKtcMZEYiOXe4u6F07ZWMNuIBN7j88jlCYnGe5Q2x1AWT2b0p6rLQe8YU7BeoS3HA/oMQtZpz9vBcYjghk8FKN4AlE7sPRnFXpkbI2oyemM2BtOrRQgE+0luJhAUnHQ0CiFb4nkUDST+8j1kiU4yhKifsRrBRTT+KGe6Wu82+801BJH59UG2tYTD46OE8/RGDwk/FQmMCSYcRGkazF2IoY1qaHoAkx9Nfr4fmyfchuTZLaUbtMv7qJSKhNfRLT06fUWTTGgyaOYs/4DqawInQxQ1g/N/qtCdJm1w01v9Fz7w/3t8ooVkzrHQiNcanESXC2GtieiBDS0ZMxkBTSUPOz9fRYy6tr7slSNXarRurKUii1Ltp9S0AegSsEj48Ibp1oBdNwns/0jibqV1MyNB/XnWRW3h4o7mhVjhHn+Kik9eWlh76oUmsw/6xS3eaw6f5nowLuvsN3VCfJ6scPvrNV8JSgNdci7z6LCfNeNob8HfvWWDVgAPHlwcV1/lX25FJNCdbm/KTJUBEdDWF2Q== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MWHPR11MB1629.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(396003)(136003)(346002)(376002)(39860400002)(366004)(451199015)(66476007)(66946007)(66556008)(6916009)(8676002)(5660300002)(316002)(8936002)(2906002)(82960400001)(41300700001)(38100700002)(6666004)(6506007)(107886003)(186003)(6512007)(4326008)(53546011)(26005)(44832011)(478600001)(6486002)(83380400001)(86362001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?H4ZGlrXAmxcAU8EbHSxv+lZau5/5iV/PI5bQiMZHxK40wLZca5JYex5Rba+0?= =?us-ascii?Q?zytRatkiQ/6rIAvLGPfwg8FL5i50zKLHarvo1rTA0+ulYZTEMGtEdStZRAlS?= =?us-ascii?Q?evTR6PiN3QHLNx4gcWMNL4ZeH56kprcYLI9HLUf/sDrRpmR7M+bJ1EuvbT3d?= =?us-ascii?Q?z+qNXRPiQe0YdLLNNVhhGMY3M+wwzdOw59ide598JJyvgItxalytvyBFeIIX?= =?us-ascii?Q?6thzWvVou0S3w5tbxB+yvL7TBBvLvs65X7JHqw+tpfnBJ4/vlVxY3RDIaWZI?= =?us-ascii?Q?h5YbZ2lXHXNYBH0LRXbzPtiNfKjnqJ7yctav9VmEga9YFBTao7uUqv9tARy1?= =?us-ascii?Q?D/gES/doGxRd9STUQnFez/lX7AEx0xuhTxFOavkGQG8SzzenoVouxz/f0ylL?= =?us-ascii?Q?6fJ9Em86oCARz1vgKZg7KQn8yac+zpbNRjkl0wKGq0KEUZjzncXGycuFW16F?= =?us-ascii?Q?6XC3n36nIvgiRj+qKFhhqsvsvooiCaDDZrBR8xt70xXZyi7wArDf78d6SEii?= =?us-ascii?Q?cyUAXz0PjL9+5kbKAPYaCeSztWp+nepcAY1Ynw1GH5gPF6MkF7qkW7sWx2nU?= =?us-ascii?Q?rCxFmXNWLdxs7TreIu49p9DJ4IQWbvCIYdRxBjpToDFpj96b11QpEYP45wfe?= =?us-ascii?Q?CyKsIFGS060laeS4eZhMSSpVAEfDRiyjvDZMM4eJPS4hQO3gXYFfR+8asrh7?= =?us-ascii?Q?Pw8xmbtWGT/v/cTD+E5VfLWW999PZGmbANWywqmiK6X9CVE3aUVpdJzo2ueM?= =?us-ascii?Q?bVowDSctE0f24q9kMB5iCodle3QaP8Iu9zER0CQrjkWgIIDxi5YGfjjePOLT?= =?us-ascii?Q?u9NtHZj76SyAC3USKlxkj3TS8GlpwAK4lesdHWRH9VEXVMMbNHn1vUbySJXJ?= =?us-ascii?Q?RoFCJbeWDpWKolYb81oK+uxjMxcZ5H2ZgpOomQjsI4KiGh8x0K7oChVv9miW?= =?us-ascii?Q?UyME8L1x9RCWIO02UyI9pq7zJLkJC/pnNHUS3S03wwkohYzkFL64CuX5TEuV?= =?us-ascii?Q?r32Tv5Qn5s8xZZ/ALB2TqPwKt1iZePvALnEqcSmzv2edsyxDK1xcDvnYyXq3?= =?us-ascii?Q?XLPuQtbGOIjYHtX6AKEzi7g9R1rtoUUbFzLBp5QrvSx2Bu9BJoKe8aFjXxHv?= =?us-ascii?Q?ExRUW/XA7wdisExQlqJYziVpX/TvX5mKk1o/J8x7onlyih+2v3YkXGPZZm68?= =?us-ascii?Q?v6JB/fQ2MOzo34kJJtQcFu/0EOKYR++v3UeELVcZFDlBv7HOPDk8Pp8LD7rg?= =?us-ascii?Q?Tq6Q1dt4eFN+r4Cdn2qOwhGMa/QI2Q0Mh9SFAHX1S2Miz5e1lDngr5C7oPsc?= =?us-ascii?Q?FnNuQnW3cqvPBUzemVitbkV7kUH/Dx1laR0CaPPlujG7bIPesDFG6uYrShZP?= =?us-ascii?Q?iwWdWbgl88OzFR7cYEmm5xm9ampFdO4iHzxWu8D7/PGVKYj08KuZjUfvJwhC?= =?us-ascii?Q?gDo+u0nHQxf7thSA5veJeQqfYwcM4vWY742K2yKEVZ11UrdYAh3YBLXQrMJf?= =?us-ascii?Q?PDE6pAaDgS/fUz9APk90zyteyPAoq3aqj6iCqSW2nSUVUJ83RuKfTaDkCGLG?= =?us-ascii?Q?8RB/Id9wGezKEQHfSQPSAWor/RKxmyIIbGl+2NOum/CFWTsCQm+Iqekbb3oF?= =?us-ascii?Q?5qgv2ok2wdYWy12W4Clhi0U=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: 9f9cbc56-fa81-4f5f-1415-08dab1f1ad46 X-MS-Exchange-CrossTenant-AuthSource: MWHPR11MB1629.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Oct 2022 16:47:58.1288 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: JS30jjcruXl4hfSPIvjfwKxEfYyit+YnnRfyOIPToxQlS52qPZUGyJy9QSJn/mHkyXTevzvApv0qKBNvkjvtPRT5xhjkuuswlR9bZwp9FgY= X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB7094 X-OriginatorOrg: intel.com 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, Oct 19, 2022 at 04:28:58PM +0200, David Marchand wrote: > On Wed, Oct 19, 2022 at 3:48 PM Bruce Richardson > wrote: > > > > On Wed, Oct 19, 2022 at 09:37:02AM +0200, David Marchand wrote: > > > Add the boolean type RTE_TEL_BOOL_VAL for values in arrays and dicts. > > > > > > Signed-off-by: David Marchand > > > --- > > > > This patch looks pretty good to me. Some very small comments inline below. > > One thing I notice is that we are not supporting booleans except as part of > > an array or dictionary. Is it likely that we will ever want to have a > > telemetry command that just returns true/false alone? Don't see that being > > I wondered too, but then I saw that only the "simple" type string was > handled and others were not. > So I decided to skip. > > > > necessary just yet, so: > > > > Reviewed-by: Bruce Richardson > > Acked-by: Bruce Richardson > > > > > app/test/test_telemetry_data.c | 88 +++++++++++++++++++++++++++++++++- > > > lib/telemetry/rte_telemetry.h | 36 ++++++++++++++ > > > lib/telemetry/telemetry.c | 24 +++++++++- > > > lib/telemetry/telemetry_data.c | 44 +++++++++++++++-- > > > lib/telemetry/telemetry_data.h | 5 ++ > > > lib/telemetry/telemetry_json.h | 34 +++++++++++++ > > > lib/telemetry/version.map | 5 ++ > > > 7 files changed, 228 insertions(+), 8 deletions(-) > > > > > > > > > > > > diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h > > > index a0d21d6b7f..5d74212f17 100644 > > > --- a/lib/telemetry/rte_telemetry.h > > > +++ b/lib/telemetry/rte_telemetry.h > > > @@ -2,6 +2,7 @@ > > > * Copyright(c) 2018 Intel Corporation > > > */ > > > > > > +#include > > > #include > > > > > > #include > > > @@ -46,6 +47,7 @@ enum rte_tel_value_type { > > > RTE_TEL_INT_VAL, /** a signed 32-bit int value */ > > > RTE_TEL_U64_VAL, /** an unsigned 64-bit int value */ > > > RTE_TEL_CONTAINER, /** a container struct */ > > > + RTE_TEL_BOOL_VAL, /** a boolean value */ > > > }; > > > > > > /** > > > @@ -155,6 +157,22 @@ int > > > rte_tel_data_add_array_container(struct rte_tel_data *d, > > > struct rte_tel_data *val, int keep); > > > > > > +/** > > > + * Add a boolean to an array. > > > + * The array must have been started by rte_tel_data_start_array() with > > > + * RTE_TEL_BOOL_VAL as the type parameter. > > > + * > > > + * @param d > > > + * The data structure passed to the callback > > > + * @param x > > > + * The number to be returned in the array > > > > number -> boolean value > > > > Indeed.. > > > > + * @return > > > + * 0 on success, negative errno on error > > > + */ > > > +__rte_experimental > > > +int > > > +rte_tel_data_add_array_bool(struct rte_tel_data *d, bool x); > > > + > > > /** > > > * Add a string value to a dictionary. > > > * The dict must have been started by rte_tel_data_start_dict(). > > > @@ -233,6 +251,24 @@ int > > > rte_tel_data_add_dict_container(struct rte_tel_data *d, const char *name, > > > struct rte_tel_data *val, int keep); > > > > > > +/** > > > + * Add a boolean value to a dictionary. > > > + * The dict must have been started by rte_tel_data_start_dict(). > > > + * > > > + * @param d > > > + * The data structure passed to the callback > > > + * @param name > > > + * The name the value is to be stored under in the dict > > > + * Must contain only alphanumeric characters or the symbols: '_' or '/' > > > + * @param val > > > + * The number to be stored in the dict > > > > number -> boolean value > > > > > + * @return > > > + * 0 on success, negative errno on error, E2BIG on string truncation of name. > > > + */ > > > +__rte_experimental > > > +int > > > +rte_tel_data_add_dict_bool(struct rte_tel_data *d, const char *name, bool val); > > > + > > > /** > > > * This telemetry callback is used when registering a telemetry command. > > > * It handles getting and formatting information to be returned to telemetry > > > diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c > > > > > diff --git a/lib/telemetry/telemetry_data.c b/lib/telemetry/telemetry_data.c > > > index 5b319c18fb..4f81f71e03 100644 > > > --- a/lib/telemetry/telemetry_data.c > > > +++ b/lib/telemetry/telemetry_data.c > > > @@ -16,10 +16,11 @@ int > > > rte_tel_data_start_array(struct rte_tel_data *d, enum rte_tel_value_type type) > > > { > > > enum tel_container_types array_types[] = { > > > - RTE_TEL_ARRAY_STRING, /* RTE_TEL_STRING_VAL = 0 */ > > > - RTE_TEL_ARRAY_INT, /* RTE_TEL_INT_VAL = 1 */ > > > - RTE_TEL_ARRAY_U64, /* RTE_TEL_u64_VAL = 2 */ > > > - RTE_TEL_ARRAY_CONTAINER, /* RTE_TEL_CONTAINER = 3 */ > > > + [RTE_TEL_STRING_VAL] = RTE_TEL_ARRAY_STRING, > > > + [RTE_TEL_INT_VAL] = RTE_TEL_ARRAY_INT, > > > + [RTE_TEL_U64_VAL] = RTE_TEL_ARRAY_U64, > > > + [RTE_TEL_CONTAINER] = RTE_TEL_ARRAY_CONTAINER, > > > + [RTE_TEL_BOOL_VAL] = RTE_TEL_ARRAY_BOOL, > > > }; > > > > Really like this change! > > I don't remember if this form is related to some CXX standard... but I > see that no compiler complained in the CI. > > > > > > > d->type = array_types[type]; > > > d->data_len = 0; > > > @@ -80,6 +81,17 @@ rte_tel_data_add_array_u64(struct rte_tel_data *d, uint64_t x) > > > return 0; > > > } > > > > > > +int > > > +rte_tel_data_add_array_bool(struct rte_tel_data *d, bool x) > > > +{ > > > + if (d->type != RTE_TEL_ARRAY_BOOL) > > > + return -EINVAL; > > > + if (d->data_len >= RTE_TEL_MAX_ARRAY_ENTRIES) > > > + return -ENOSPC; > > > + d->data.array[d->data_len++].boolval = x; > > > + return 0; > > > +} > > > + > > > int > > > rte_tel_data_add_array_container(struct rte_tel_data *d, > > > struct rte_tel_data *val, int keep) > > > > > diff --git a/lib/telemetry/telemetry_json.h b/lib/telemetry/telemetry_json.h > > > index e3fae7c30d..c97da97366 100644 > > > --- a/lib/telemetry/telemetry_json.h > > > +++ b/lib/telemetry/telemetry_json.h > > > @@ -7,6 +7,7 @@ > > > > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -159,6 +160,21 @@ rte_tel_json_add_array_u64(char *buf, const int len, const int used, > > > return ret == 0 ? used : end + ret; > > > } > > > > > > +/* Appends a boolean into the JSON array in the provided buffer. */ > > > +static inline int > > > +rte_tel_json_add_array_bool(char *buf, const int len, const int used, > > > + bool val) > > > +{ > > > + int ret, end = used - 1; /* strip off final delimiter */ > > > + if (used <= 2) /* assume empty, since minimum is '[]' */ > > > + return __json_snprintf(buf, len, "[%s]", > > > + val ? "true" : "false"); > > > + > > > + ret = __json_snprintf(buf + end, len - end, ",%s]", > > > + val ? "true" : "false"); > > > > Wonder if it's worthwhile doing a macro for this conditional, since the > > same ternary-operator snippet appears 4 times in this code. > > Err, naming it would be hard and I don't see for now how we could reuse it. > Yes, and I see Morten has objected from a readability perspective, so keeping as-is is fine. One final suggestion though might be to have an array with the strings as so: const char *bool_str[2] = { "false", "true" }; and then in the code use "bool_str[val]" in place of ternary operator. (From a quick check with godbolt is looks like bool params are clamped to 0 or 1 on function call, but if we want to be paranoid, we can lookup based on [!!val]) However, ok to keep code as-is for this too. /Bruce