From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 9A2E0428BA;
	Tue,  4 Apr 2023 10:47:38 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 2B2D7410FA;
	Tue,  4 Apr 2023 10:47:38 +0200 (CEST)
Received: from mga18.intel.com (mga18.intel.com [134.134.136.126])
 by mails.dpdk.org (Postfix) with ESMTP id 7803E40EE3
 for <dev@dpdk.org>; Tue,  4 Apr 2023 10:47:36 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple;
 d=intel.com; i=@intel.com; q=dns/txt; s=Intel;
 t=1680598056; x=1712134056;
 h=date:from:to:cc:subject:message-id:references:
 in-reply-to:mime-version;
 bh=V96tCkcybIHeZ+UCaMZNcImRGo//zFv/EywMY286Y4g=;
 b=OKDDrX/m9p4WZUkv1UCTryohU5GcAGlbxQud1mFCFuCTI3uhonTmVY8J
 sXE1q2GuknmC0auzSCEtWYg1dVC/4mCOSQa0mxGazzhosBubDeZzRoy6O
 twLoPQHeYCKER4XheId88m/EYT2Mh4gBNsQUTVYvTQ3V6zOoVFcOu+69D
 zijLi6V6bquxEqb2K2Yq6Fr0x3oR5kbQrmh1CDTNLafhdhl11CG/3f+N1
 AKuvWJYwe0QCcRVTDASo+Ulkfb4OKaaJLBiI8yoktrqjbsZtVmqFgfI8S
 aBI6/VMPnK4pZHJR22fMNxEJ75RRTDZM49sTIp1E7Pw2QjWf8Hm+5SyWO A==;
X-IronPort-AV: E=McAfee;i="6600,9927,10669"; a="326145399"
X-IronPort-AV: E=Sophos;i="5.98,317,1673942400"; d="scan'208";a="326145399"
Received: from fmsmga003.fm.intel.com ([10.253.24.29])
 by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
 04 Apr 2023 01:47:35 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=McAfee;i="6600,9927,10669"; a="775560843"
X-IronPort-AV: E=Sophos;i="5.98,317,1673942400"; d="scan'208";a="775560843"
Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16])
 by FMSMGA003.fm.intel.com with ESMTP; 04 Apr 2023 01:47:34 -0700
Received: from orsmsx611.amr.corp.intel.com (10.22.229.24) 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.2507.21; Tue, 4 Apr 2023 01:47:34 -0700
Received: from orsmsx603.amr.corp.intel.com (10.22.229.16) by
 ORSMSX611.amr.corp.intel.com (10.22.229.24) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
 15.1.2507.21; Tue, 4 Apr 2023 01:47:34 -0700
Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) 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.2507.21 via Frontend Transport; Tue, 4 Apr 2023 01:47:34 -0700
Received: from NAM12-DM6-obe.outbound.protection.outlook.com (104.47.59.170)
 by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.1.2507.21; Tue, 4 Apr 2023 01:47:33 -0700
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
 b=gPC4j3LYu2fjPnNTsd7ymTpEuoQmd/Z16rYl/8WPZbrd3QjHqdTMdUxXBXG5rjaIzoQ/D3xJ67UgNd9A/5IF4e+LjMZz+pvBPkPcTt+sPqdHIMq24DbTtU1MTNZWYteYNTuUrynDmSAiR3Y6fe9Q5tb2AiDByUfB2bkq0/MDkC1eG6cxRj5qyXPo85dvY5YGlI8u2Wa+3Eb02TYmFHCQx/fqr9KByt0xb8nW3UcaPkflcuWGMB8U+v8DRnUkbKy1aPLZebgXw4PooYxPUhXDkyJpeI7mBhuZmFVjR9rDrK/hBtCw5+o0P08kezWRPMHJ5lrRv3LqvIPoFh0w8Dljeg==
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=jkTajZlKzlPpLs3B6kjImo8lSd7QCigov4nWY3s+MCs=;
 b=bN7HxKMdaQixHmxrCoOAzDASqfKcrWtOwgResRzqIO8T3lA1WIbHz/6/S1uZIetXk7zlnIinRwu+V8sYeDFQbVOVXPUz7PylspmBtwn0Fdik6ja0rjdG0WPQzkBNCQKT4c92ENgoUsbtfCBkrO834JBMyTJLcg0KSts9OBOjl80SstxP5Rf+X4o+upISEiOU1C8jeODVuIYoCchQSQ16sznRXJQv+lV5FcNQHet+zhQtDynw5PxYoLkeEKM7Psubl8d2b+4JJuv08DetfM72WGG+BtJ66nWyu2WNmNGM6yC2fOo3/v7NGRUAmrp/dMvimhJIfkOsMXbUZeVbKh2CBg==
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 DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17)
 by DS0PR11MB7971.namprd11.prod.outlook.com (2603:10b6:8:122::12) with
 Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6254.35; Tue, 4 Apr
 2023 08:47:27 +0000
Received: from DS0PR11MB7309.namprd11.prod.outlook.com
 ([fe80::18d0:ac53:aa1d:d19c]) by DS0PR11MB7309.namprd11.prod.outlook.com
 ([fe80::18d0:ac53:aa1d:d19c%6]) with mapi id 15.20.6254.035; Tue, 4 Apr 2023
 08:47:27 +0000
Date: Tue, 4 Apr 2023 09:47:21 +0100
From: Bruce Richardson <bruce.richardson@intel.com>
To: Stephen Hemminger <stephen@networkplumber.org>
CC: Tyler Retzlaff <roretzla@linux.microsoft.com>, <dev@dpdk.org>,
 <ciara.power@intel.com>, <david.marchand@redhat.com>, <thomas@monjalon.net>
Subject: Re: [PATCH 1/2] telemetry: use malloc instead of variable length array
Message-ID: <ZCvkGY8J0qakKfqX@bricha3-MOBL.ger.corp.intel.com>
References: <1680539424-20255-1-git-send-email-roretzla@linux.microsoft.com>
 <1680539424-20255-2-git-send-email-roretzla@linux.microsoft.com>
 <20230403131913.0aec54ce@hermes.local>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230403131913.0aec54ce@hermes.local>
X-ClientProxiedBy: LO4P265CA0072.GBRP265.PROD.OUTLOOK.COM
 (2603:10a6:600:2af::21) To DS0PR11MB7309.namprd11.prod.outlook.com
 (2603:10b6:8:13e::17)
MIME-Version: 1.0
X-MS-PublicTrafficType: Email
X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|DS0PR11MB7971:EE_
X-MS-Office365-Filtering-Correlation-Id: 13c53c74-e6a3-4af1-dae5-08db34e937c6
X-MS-Exchange-SenderADCheck: 1
X-MS-Exchange-AntiSpam-Relay: 0
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: uPdgVTxTu8azLVyQCfWRqu+QBWs3w+8qSxz+9HGl4rEqlDYBAobwdLuRrhhekv20LzwWhW+Pa9V8lnPB/FfLAPAYW7rNrPlp8TBhwREJfOldBXZquP6FnjCIjepytWIzmHHlmigEqe8d6u2w8NLx2T7w5NTzWbUcPwwxNnlD+UUzgxWUVIL7pdHtA3GuMhCbpThAl7dVu4MC91K5eiBd6liXtmiQ86KoRMUCCz1Tv0so1k9SlFmMLz20r3Tztbp5SsmHq/hNslgW0jjdzPhYJT67SzDoQN9Lh8vMYX+AAa32tVMnrAvFVcX1wNbes4576yK3cc747MkL3Zm6bQgwsetM0DlGc9dKVPreX2LQzaYh2vYg6l2Qlt+yly9vJwhNpJRWS9PEjrZ5sS/GoCqGeevUvQG003T9hvmPoWKJOT+dAvcfQF4gF466S4B2AUnuiakSuzM26hibM8Z2GJ9RewPTzpR1VQLVWuNsuLNqlENmVG78dAUafyutvtO2LKD2O5NbteskT/Zd3nphEHs3wgIha43MAgXrIyKGgPdhJx/wRLVSvuXMqvjnwjdWJdl5
X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;
 IPV:NLI; SFV:NSPM; H:DS0PR11MB7309.namprd11.prod.outlook.com; PTR:; CAT:NONE;
 SFS:(13230028)(39860400002)(346002)(136003)(366004)(396003)(376002)(451199021)(86362001)(2906002)(6486002)(186003)(83380400001)(6512007)(26005)(6506007)(6666004)(4326008)(8676002)(66946007)(66556008)(66476007)(478600001)(41300700001)(82960400001)(5660300002)(38100700002)(6916009)(316002)(44832011)(8936002);
 DIR:OUT; SFP:1102; 
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?BB4Teve/tzZVQi2KDadmuI7OvJDXBnGN7y0J4+lOIpIqGMVfH1AcBK3NISv5?=
 =?us-ascii?Q?1Tclxl1RKykAvwI9sLVQbZLo+ja3ac0sm0Jk/JwruZtj3CgMTpII/+VK4uIx?=
 =?us-ascii?Q?3ehDz+N5riBDiwn21iI0rKXw/thflkGZfjJgGHsbYU2cVL3fcrnQGJwEqDoa?=
 =?us-ascii?Q?Kvf3Ew4+mkINZ8wBszAbX7zJLMQFtsGChQvjpTfPN1ngs4/QfG70qOW4DYFf?=
 =?us-ascii?Q?eG6LbJBUfiEX1WcoQOqgFM5gl2Q9rODTcCyJbXGSOdrvk1PHuoWOSdWnn+gB?=
 =?us-ascii?Q?RdhW9N4O+qQlmGOP0BsNYWEb57VufOQCaOzpnAVW8YmdMGZNbcoe9vyc/Ccj?=
 =?us-ascii?Q?fT/9yX6msGIjFZVVNTAOIm5bjedLw7/3AA2pH1PsjqdiXMRUieVa7NP5ACiX?=
 =?us-ascii?Q?IYp1L20AQsvo55xTk9+EWyCd6o+G6xvUNw0DXrwjce5wZeu64cSu6pLu/YGS?=
 =?us-ascii?Q?121gAab2ygBnADdjYiyBb+CR+0o5ynUpU4NllVniMCrOAwv1NBiq3Qaj5+ol?=
 =?us-ascii?Q?M3YdVida6RE4c7tobMU0MZUSTA4c9L4T194ZTrfDNrF2+PSKnTNZYTYDNGJK?=
 =?us-ascii?Q?BkwyP1UwWIQqLV/VgIu0/KFImYLayE0MQNyRmD649LBNyfmwOPXiv8M1DDdE?=
 =?us-ascii?Q?NcCma/y3OVU6yLRYu6iu9Igjoy7SzTpBD+Lt4Z17LrtKRpwSrYzM7P54CWVD?=
 =?us-ascii?Q?BlL5UwdDLhcH/cCSDFLR80AM6gMydaN/vOqyW0nQoqhTE8Uy3t7yYXZU4kmp?=
 =?us-ascii?Q?cz5EhG/eGgbUYTMb5nWANETK+LA2QgK2jlxStcLvmPnPFXmvGDIvuuqeQQ7N?=
 =?us-ascii?Q?qbu+bUhoxwYuXagSq3b8HDvsxoEkuXiWLzS5kBXbsZRJBPc0DxLQnisBqNwU?=
 =?us-ascii?Q?vVfwAGtGkXU6FUgFCBWcgdzOCt0zo6pzomGiJareoYkCDUbGYOa32uZDE4I1?=
 =?us-ascii?Q?Z4OxYOMI0INJ7EoI0JqsxDh8EYwT3yA0ez7bwfGIcE7olFdBRz+wpCjSbV/J?=
 =?us-ascii?Q?MjHoL6rh6aHnhMSPaXy/3luzfeMm5MS7VvK9HSdyzq+BPFvWaWAMVqh2Vpxa?=
 =?us-ascii?Q?LjUhQ64nCOgLbY6MxCEGAeMfDha/pC2Gasjnjn/AQhhGiKjO2Vxs26sJXr9R?=
 =?us-ascii?Q?/1iBGp0SpTAWS0vP5EUOo+P2ObY8+lGLIIr9Xyf0CLxUmzkr5lve820MZGY3?=
 =?us-ascii?Q?wMprkSU/l15paeTRoLmm8YH3PbWn8UR4GivfEtO0Q0UMvW3V+EvMTNLb8DKt?=
 =?us-ascii?Q?8uG8SYwgcB2hU7fMcAJUCAkjbKHOrpZY/ZfjhfhBJ22BwqePVxdMPlNaNdg6?=
 =?us-ascii?Q?GnrPReagItOdH3O1xUjRASADv8ud0E0CMrHXbW7zHOVV3PSZ17/YenpI8xs/?=
 =?us-ascii?Q?TMmtWLP/FPR0+1oUJDDea6CFFnlRaKz9Mtjo7O78HjyBCu7zVZ7+o2mjVl4Z?=
 =?us-ascii?Q?XTGyrklnuAYeVuAAnGQdUhu4icPP/zXflHXrj1TBFh+wToJ7LldAzU26v8oM?=
 =?us-ascii?Q?YPUnniNS1PUK8npMUZwjMdtQO9m/zengH4cBLdgx007IKa+RbvZmQYxIlSO+?=
 =?us-ascii?Q?9fn6NDz1Xt2/FeAm1G4IQreN9lHWAHBTFMFrXfFQEWByEQHlf5oFovByFIo9?=
 =?us-ascii?Q?wQ=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 13c53c74-e6a3-4af1-dae5-08db34e937c6
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Apr 2023 08:47:27.2688 (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: M4oiTVnuXAz22jOpYLBHIjM9E6updeAO6V4B3X9P3Fwy9I9djPec9JLDOCCTXbqBxFDzeQJCp90Sc1QasLrM9G3UuwmCbw3zcib4IYwKLl8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB7971
X-OriginatorOrg: intel.com
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

On Mon, Apr 03, 2023 at 01:19:12PM -0700, Stephen Hemminger wrote:
> On Mon,  3 Apr 2023 09:30:23 -0700
> Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:
> 
> >  __json_snprintf(char *buf, const int len, const char *format, ...)
> >  {
> > -	char tmp[len];
> > +	char *tmp = malloc(len);
> >  	va_list ap;
> > -	int ret;
> > +	int ret = 0;
> > +
> > +	if (tmp == NULL)
> > +		return ret;
> >  
> >  	va_start(ap, format);
> >  	ret = vsnprintf(tmp, sizeof(tmp), format, ap);
> >  	va_end(ap);
> >  	if (ret > 0 && ret < (int)sizeof(tmp) && ret < len) {
> >  		strcpy(buf, tmp);
> > -		return ret;
> >  	}
> > -	return 0; /* nothing written or modified */
> > +
> > +	free(tmp);
> > +
> > +	return ret;
> >  }
> 
> Not sure why it needs a tmp buffer anyway?

The temporary buffer is to ensure that in the case that the data doesn't
fit in the buffer, the buffer remains unmodified. The reason for this is
that when building up the json response we always have a valid json string.

For example, suppose we are preparing a response with an array of two
strings. After the first string has been processed, the output buffer
contains: '["string1"]'. When json_snprintf is being called to add string2,
there are a couple of things to note:
* the text to be inserted will be put not at the end of the string, but
  before the closing "]".
* the actual text to be inserted will be ',"string2"]', so ensuring that
  the final buffer is valid.
However, the error case is problematic. While we can catch the case where
the string to be inserted overflows/has been truncated, doing a regular
snprintf means that our output buffer could contain invalid json, as our
end-terminator would have been overwritten, e.g. '["string1","string2'
To guarantee the output from telemetry is always valid json, even in case
of truncation, we use a temporary buffer to do the write initially, and if
it doesn't get truncated, we then copy that to the final buffer.

That's the logic for this temporary buffer. Now, thinking about it
yesterday evening, there are other ways in which we can do this, which can
avoid this temporary buffer.
1. We can do the initial snprintf to an empty buffer to get the length that
   way. This will still be slower, as it means that we need to do printf
   processing twice rather than using memcpy to copy the result. However, it's
   probably less overhead than malloc and free.
2. AFAIK, the normal case for this function being called is with a single
   terminator at the end of the string. We can take advantage of that, by
   checking if the '\0' just one character into the string we are printing,
   and, if so, to store that once character. If we have a snprintf error
   leading to truncation, it then allows us to restore the original string.

My suggestion is to use a combination of these methods. In json_snprintf
check if the input buffer is empty or has only one character in it, and use
method #2 if so. If that's not the case, then fallback to method #1 and do
a double snprintf.

Make sense? Any other suggestions?

/Bruce