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 2DE72A0545; Thu, 15 Dec 2022 14:08:33 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C0E3240684; Thu, 15 Dec 2022 14:08:32 +0100 (CET) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id 581E640223 for ; Thu, 15 Dec 2022 14:08:31 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1671109711; x=1702645711; h=date:from:to:cc:subject:message-id:references: content-transfer-encoding:in-reply-to:mime-version; bh=PRv2u4rXZXhK1l7RPcFpxeL/WeH8twAt33UMalgg6aM=; b=fcHlqLRqDMbMdqq6e0CsAN1KX0C3F0tVG1AxQXCt2r/BU0YPDjsMI9OR i3XJg0HAi/+dz98I6G774rJctWD7tkpjWOLN9PVbEdWV0Im2olCRmU3FM VszNHcFta4BqDeTApp0gOYpngyZ5/obvt+ZCoH2z3dKnEaGPEGqnmNlne 1tiEEn+FAzQ1M+zSj/wg+IFHydC4HQcD8UNm2SPqCWIPynKfn/lx41wef p4blaLWSRJiILzrodYbiudAjrYJlZsHacNEtT8KociWwLZmnEbUJKBjt1 dY29kDiN2kX3PK3BkHOMc/+N8mX43qwbOQBGavMqLRonLUu98n0gah+zE A==; X-IronPort-AV: E=McAfee;i="6500,9779,10561"; a="316307249" X-IronPort-AV: E=Sophos;i="5.96,247,1665471600"; d="scan'208";a="316307249" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Dec 2022 05:08:29 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10561"; a="791624564" X-IronPort-AV: E=Sophos;i="5.96,247,1665471600"; d="scan'208";a="791624564" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by fmsmga001.fm.intel.com with ESMTP; 15 Dec 2022 05:08:29 -0800 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Thu, 15 Dec 2022 05:08:28 -0800 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx612.amr.corp.intel.com (10.18.126.92) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Thu, 15 Dec 2022 05:08:28 -0800 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx611.amr.corp.intel.com (10.18.126.91) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16 via Frontend Transport; Thu, 15 Dec 2022 05:08:28 -0800 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (104.47.55.101) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.16; Thu, 15 Dec 2022 05:08:27 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=lE8yrw+Nkgad+hi5CPMHgKODT+KIyF5hN0leI+YwvA3hLG+ysQZeTVcJfQAGk8VA2DrZ4ODXUgHn89+vclwXxxaYj9lW0W2egXs8Zcv4oG3bddQ8EK6Qgt2NhuGevtErIjuwtcqkdWap7R4CbGJy0VUSVa/geYDjKS/+lBcgE28lEB1eYuOUpffHE6fZvYzv0O/XKdCrFObl5jrXK7VFmInxnkWZnozvQ24jSAyNCFbrK0pxFEkEjh3VCEngMyBana6LF1+bmQ3T0XUBmV3xlyWRz/zHFGPp8dsrtaprq/joyiGe14bIKbgNhw51WQ86iKIKDBPrIB3avXZO1FVRcA== 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=wtuYJaM2xUWGCweCkg0cOZ9vdLMTNMcJpZ4+d6W2peo=; b=HuElrvjV36lw/r1a72tIX1MdgHctwMyt6ubbTWdcfq25eOUOCNqaM2d0d43nMBYHW4Feeclrzd0hVozQXr5w6k7upMo1Sf8nIowA5Ld7LpI71B7Tpr0q2P8aArrVbS2kHSUyDs/jjBZtegvk9fD9+sGcAIa+TsX+y2/n1nr6AP/PvMFVAF/cmSKzqCK+geXu9qEXOWJjFzO9Za7HzKywqV8OesqqzUT7r/CaQLsf3p3pGGXX2WYxLLiKH+fGXHUdZ2zSjb8HYJLdNlzduyzeLPQ56vbClcr7s20pZLuSVLb4UfeczTrgQ0TCOgWmaeIw46mCq7jF4zSnyt8bT0T5ug== 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 PH7PR11MB5768.namprd11.prod.outlook.com (2603:10b6:510:131::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5924.12; Thu, 15 Dec 2022 13:08:25 +0000 Received: from MWHPR11MB1629.namprd11.prod.outlook.com ([fe80::18bd:edae:ad31:a228]) by MWHPR11MB1629.namprd11.prod.outlook.com ([fe80::18bd:edae:ad31:a228%3]) with mapi id 15.20.5924.011; Thu, 15 Dec 2022 13:08:25 +0000 Date: Thu, 15 Dec 2022 13:08:18 +0000 From: Bruce Richardson To: Morten =?iso-8859-1?Q?Br=F8rup?= CC: "lihuisong (C)" , , , , , Subject: Re: [PATCH V6 6/8] telemetry: support adding integer value as hexadecimal Message-ID: References: <20221208080540.62913-1-lihuisong@huawei.com> <20221215103146.816-1-lihuisong@huawei.com> <20221215103146.816-7-lihuisong@huawei.com> <91c7e257-9c7c-849e-c768-cd48a6e659a3@huawei.com> <98CBD80474FA8B44BF855DF32C47DC35D875AD@smartserver.smartshare.dk> <98CBD80474FA8B44BF855DF32C47DC35D875AE@smartserver.smartshare.dk> <98CBD80474FA8B44BF855DF32C47DC35D875AF@smartserver.smartshare.dk> Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D875AF@smartserver.smartshare.dk> X-ClientProxiedBy: LO2P265CA0045.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:61::33) To MWHPR11MB1629.namprd11.prod.outlook.com (2603:10b6:301:d::21) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MWHPR11MB1629:EE_|PH7PR11MB5768:EE_ X-MS-Office365-Filtering-Correlation-Id: 354d7d44-948f-45d6-9958-08dade9d7344 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: L09bibrP28eyl2uaCBtV9MeJaQuiP8k57Xpn1EdUukwibt2KCQSbRAkikFsJWyA3ewEOtYUhoQK7ubAegFQfSjHL93EtZjuB0JmYzwmDLNQlwzcrqb2iPfk5/WPzLO55onVcIk+wmT76ntCkMKkpb4U4gm778xZSn4oi9Aqvsms+Xhew5N+XYUL5tFvmvH2j902CPHJqS7wVAPzBmAmn87j4n8UJvkMOsKfKdN5pdtUu8i2sPiu8jYBK5X3aB+rkdNFfcwUogeqFLQqB6GCRIxkiG+OcEhFppgZEQOXWMJsh7OlEr+0oNZ8+6Yv7XaGkTfUit7qh2BptkzU0dtQ83cmNaHZpqPeDP41dQYDub0GRTrleI1aCU3PlEd0e5A6Z8YwTA9gH6/UxQ3d/g0mXgh9mykzuyfZvdevyiJ444kYCsLo8kGXCLWJoazT+Fa+kJPJO/t9U7V9fMarTGJP+6Q+u2h+fIPwTBha4Lzo9X4v0Hry4l9s5dFhJu/7KvwufcK/bHwyw6msV8LUUx3xKFjnl2GRfAQrym4oWm5+uaJCQ9VcKwPJ834mY7WadsrcG0eXWEk4l2H2ppweigVt71h9eX0B0bq1wbcQ2Mlw5uauHEEH1IMabhdHy721JMOUAuh30C2K0yLr/FAXtLeBN3A== 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)(346002)(376002)(39860400002)(396003)(136003)(366004)(451199015)(6916009)(41300700001)(316002)(83380400001)(8676002)(4326008)(66946007)(66476007)(66556008)(82960400001)(66574015)(6486002)(478600001)(86362001)(186003)(6512007)(26005)(38100700002)(6506007)(6666004)(2906002)(44832011)(8936002)(5660300002); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eXdHTzdHNG0rY3hHZFZPNmlLNUJvbW41QzlnWUpHa0t1TUxBSGV0SVNORytt?= =?utf-8?B?dG5nTE5sNklLNENOdGV5RHFJeVZYV0lVbk9nd09nZU8ycHlaSk5pMzNWN2Z6?= =?utf-8?B?dHV4dzhRR0UrL0dKbGlmK3Y0S3ppMWNmV0xxZ3BUVFJXRVFFV3BGek1xMDFL?= =?utf-8?B?QjRXdHFFRDdJbDZNR2N2Q3Ewb3Z3THlRbG5STmgyWDNCWGErendqUXJOV083?= =?utf-8?B?eExYOVNraUxiTTV2OW1vNVFGdkVHK0wxbUFEd2FzZng3aThZczB4eUY5Y3J2?= =?utf-8?B?U1czM1ZnNHBIbTBMaTFXQUNtUHdPNHdvUEw5US9IVjloZ1lVSGZMVWdCQlR1?= =?utf-8?B?dGtlajBVWUZPellHdzE3T2lORllCc2pqOURad1liMmdqSlNVZ2pPcGVoTzVV?= =?utf-8?B?RlBMZkUvYTYyTlVvT0I1d0oxUmpCajFlMTc1UTdiTWJ4UXl2Y1hnTDFiTHFU?= =?utf-8?B?TUU0SXplNTZta1dzaUYrVmhNcUFsUlJEaDFhSnpyUHRhRER1U2t4cFo0Nlhl?= =?utf-8?B?WW1qS2d2M3piN2UvZEU1L0xqb0NsZmFYVGg3bElhTFNRcXpWQ2U0MFFQSmw0?= =?utf-8?B?dFB1aDJid0UvSWhZbldjMWVwN1RyMjRLL0tzekQ4cTZnYmtzOFU4VFR0U2tl?= =?utf-8?B?MVdqZU5Bb01vZHlobGpYaUhiQ1lLY3FMTk0vYU55SEJEUWtpeU9jVXhIallk?= =?utf-8?B?N2JtUDVYSDV1dU9RNVpQWm8reG9UYWJEbjdyc0pKRHNpU2h2ejAwSGxyZStl?= =?utf-8?B?WExYemZyZjJvZE1BQmoycWNBRTJMN2hXZjVPdG0za041aEc1c0NkU3lxbUZW?= =?utf-8?B?a1RQMTNXcEVKaGNnQjUzQnJpeTF3TjVXYjhsN2ptSzlEY2F0VzVtZU5QMHVv?= =?utf-8?B?ekxaQ3FTUSt5T0ZDU0pNS3FUalJzL3l1VkFnTnNwMTAxZDlzc21oTDJMcGFx?= =?utf-8?B?WFNjaitsYUNFSGVGMGh1UFVDZ3pGUzRyVzFueXdIUVhRZlpUVmtoUWVZdE9H?= =?utf-8?B?T0llWlhaZTZKY05NejNvVjZ6K0VNRHlYbHk2RDZRUkRxVW5DeVhwYUxJbVBQ?= =?utf-8?B?NUdRMUQ0UHUrVTJRRmREd3Rva2JFVU9ydGlFVnpYSHA0VFJCRWdJK0gwMEF0?= =?utf-8?B?NU51eS8wU0JSZlh5aXI0MzNsQlRJdU5lVm5QQU9hVnJxSjNnVnNoYzY3UGxJ?= =?utf-8?B?V3YrcXplZ2tnV0pTSUJYL3BVYUNNVjVnNUxIYUI1M01jMDVIMEZ0R004Z0RK?= =?utf-8?B?bjRNeThTbE5CbVhTc2hIdkc4eXlWeGMzNlZlZjRJNEY0cGQ5Unc0RWJ1Vjcy?= =?utf-8?B?ZDFWNjh0bFdvTmd4R3puN0p5Tys3YkNIbWhUNVF3bWNUNVBEb0RvTjNXQjNl?= =?utf-8?B?eWJocWRiS1hVZFpPTEp1QmVaSm9vMDRBNERFMyt1UVQzUTNUNTZsOHo3bHBG?= =?utf-8?B?M1hHVHU0eS9YNVU4TnpFenRHeGZwSmtpMTNXeGZtNDdYNkRQUGdRejg2Qk1S?= =?utf-8?B?THpTOFA2elljZjBGWXlIVTEyWFk1M2hvbjVNb29qTVpOVlNrOFV6eDNiZTJV?= =?utf-8?B?aFJSdklobVNqY0pTdFVUUkE4RUtTM1dZWXRCZDU0NDdjWDJNU2ZabEhEZVIy?= =?utf-8?B?bVlBcWU4VHlLQVFudmxXTHpBNE5NaDZQNGFCQmM1Z2h4RGFjZ3gwaGVyVzE5?= =?utf-8?B?ZTJ0blVldGdKQlBWelhvNW8rT285cTVKc01uemVxZWl4UkROMGU5Q3ppUTlv?= =?utf-8?B?akE5SXpLYlQ3cFhIdVJmb0p6dmVueHprV2o0bW1rY0poekQvNU45YmN1enN6?= =?utf-8?B?SlEyVEg4VUhHb0dRTFp3a1R1eFRqU3hKWWxWTkdHeHFYVWtnQXUvQ1J2d1ZV?= =?utf-8?B?MThmajZmS1V4QjRBMVlqUTBxU3JWdkJybi9YczUxeFVHK3NGbHRBRDlTMmpG?= =?utf-8?B?KzBSMENtajNHTE9Oeko3RVNxT05DSTNJTnlqSEphUXhVMGxTRTZvYjFMV0VP?= =?utf-8?B?RytZWHljOXFNbGY3MHBrdUVFU05VS3ByMG1ENmIrTG53WTF2M0pYcThkQmVw?= =?utf-8?B?ZmZPOURtYmdHK1Z3SnZCUHZaYzRTbTZpakpCSFVzK1Y0blNMMWNkZnFiemhm?= =?utf-8?B?V0srVEVlM2tGaFY4RWsvZnBIWW1JNzdoaDVHZUh0M1lNRE1tNFVEek5NQWlq?= =?utf-8?B?Snc9PQ==?= X-MS-Exchange-CrossTenant-Network-Message-Id: 354d7d44-948f-45d6-9958-08dade9d7344 X-MS-Exchange-CrossTenant-AuthSource: MWHPR11MB1629.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Dec 2022 13:08:25.3953 (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: tL42phV1j7osKPDWSom3O5MXoqbFcr3BjBfppaCLb8f2B/C9MZ5R16mp+bmlithva+/DS4u9SxGKD1WZJDmQjfRFTS9AleajulmgE5gxWjo= X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB5768 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 Thu, Dec 15, 2022 at 01:52:02PM +0100, Morten Brørup wrote: > > From: lihuisong (C) [mailto:lihuisong@huawei.com] > > Sent: Thursday, 15 December 2022 13.46 > > > > 在 2022/12/15 20:24, Morten Brørup 写道: > > >> From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > >> Sent: Thursday, 15 December 2022 13.16 > > >> > > >> On Thu, Dec 15, 2022 at 01:00:40PM +0100, Morten Brørup wrote: > > >>>> From: lihuisong (C) [mailto:lihuisong@huawei.com] > > >>>> Sent: Thursday, 15 December 2022 12.28 > > >>>> > > >>>> 在 2022/12/15 18:46, Bruce Richardson 写道: > > >>>>> On Thu, Dec 15, 2022 at 06:31:44PM +0800, Huisong Li wrote: > > >>>>>> Sometimes displaying a unsigned integer value as hexadecimal > > >> encoded > > >>>> style > > >>>>>> is more expected for human consumption, such as, offload > > >> capability > > >>>> and > > >>>>>> device flag. This patch introduces two APIs to add unsigned > > >> integer > > >>>> value > > >>>>>> as hexadecimal encoded string to array or dictionary. And user > > >> can > > >>>> choose > > >>>>>> whether the stored value is padding to the specified width. > > >>>>>> > > >>>>>> Signed-off-by: Huisong Li > > >>>>>> Acked-by: Morten Brørup > > >>>>>> Acked-by: Chengwen Feng > > >>>>>> --- > > >>>>>> lib/telemetry/rte_telemetry.h | 47 ++++++++++++++++++++++ > > >>>>>> lib/telemetry/telemetry_data.c | 72 > > >>>> ++++++++++++++++++++++++++++++++++ > > >>>>>> lib/telemetry/version.map | 9 +++++ > > >>>>>> 3 files changed, 128 insertions(+) > > >>>>>> > > >>>>> > > >>>>>> +/* To suppress compiler warning about format string. */ > > >>>>>> +#if defined(RTE_TOOLCHAIN_GCC) > > >>>>>> +#pragma GCC diagnostic push > > >>>>>> +#pragma GCC diagnostic ignored "-Wformat-nonliteral" > > >>>>>> +#elif defined(RTE_TOOLCHAIN_CLANG) > > >>>>>> +#pragma clang diagnostic push > > >>>>>> +#pragma clang diagnostic ignored "-Wformat-nonliteral" > > >>>>>> +#endif > > >>>>>> + > > >>>>>> +static int > > >>>>>> +rte_tel_uint_to_hex_encoded_str(char *buf, uint16_t len, > > >> uint64_t > > >>>> val, > > >>> The "len" parameter should be size_t or unsigned int, not uint16_t. > > >>> > > >>> And as a personal preference, I would name it "buf_len" instead of > > >> "len". > > >>>>>> + uint8_t display_bitwidth) > > >>>>>> +{ > > >>>>>> +#define RTE_TEL_UINT_HEX_FORMAT_LEN 16 > > >>>>>> + > > >>>>>> + uint8_t spec_hex_width = (display_bitwidth + 3) / 4; > > >>>>>> + char format[RTE_TEL_UINT_HEX_FORMAT_LEN]; > > >>>>>> + > > >>>>>> + /* Buffer needs to have room to contain the prefix '0x' and > > >> '\0'. > > >>>> */ > > >>>>>> + if (len < spec_hex_width + 3) > > >>>>>> + return -EINVAL; > > >>>>>> + > > >>>>> This check is not sufficient, as display_bitwidth could be 0 for > > >>>> example, > > >>>>> and the actual printed number have up to 16 characters. > > >>>> Yes, you are right. What do you think of the following check? > > >>>> > > >>>> max_hex_width = display_bitwidth == 0 ? (sizeof(uint64_t) * 2) : > > >>>> spec_hex_width; > > >>>> if (len < max_hex_width + 3) > > >>>>     return -EINVAL; > > >>> LGTM. > > >> That would work, but actually I think we should drop this check > > >> completely > > >> - see comment below. > > >> > > >>>>>> + if (display_bitwidth != 0) { > > >>>>>> + sprintf(format, "0x%%0%u" PRIx64, spec_hex_width); > > >>>>>> + sprintf(buf, format, val); > > >>> snprintf(format, sizeof(format), "0x%%0%u" PRIx64, spec_hex_width); > > >>> snprintf(buf, len, format, val); > > >>> > > >>>>>> + } else { > > >>>>>> + sprintf(buf, "0x%" PRIx64, val); > > >>> snprintf(buf, len, "0x%" PRIx64, val); > > >>> > > >>>>>> + } > > >>>>>> + > > >>>>> snprintf should always be used when printing to the buffer so as > > >> to > > >>>> avoid > > >>>>> overflow. The return value after snprintf should always be > > >> checked > > >>>> too. > > >>>> If check for buffer is sufficient, can the return value of > > snprintf > > >> not > > >>>> be checked? > > >>>> There are also many places in telemetry lib that are not checked > > >> for > > >>>> this return value. > > >>>> Do you have any principles for this? > > >>> You already check the buffer size before printf() into it, so I > > think > > >> it suffices. However, to keep automated code checkers happy, you > > could > > >> easily use snprintf() instead of printf(). (Sorry about not doing it > > in > > >> my example.) > > >>> I somewhat disagree with Bruce's suggestion to check the return > > value > > >> from snprintf() after checking that the buffer is large enough. It > > >> would be effectively dead code, which might cause some confusion. On > > >> the other hand, it might keep some automated code checkers happy. In > > >> this specific case here, I don't have a strong preference. > > >> Sure, you don't need 2 checks - we can either check the length at > > the > > >> start, or else check the length by looking for the return value from > > >> snprintf, but we don't need to do both. > > >> > > >> Given the slight complexity in determining the correct length of the > > >> printf > > >> for the initial size check, I think I would go with the approach of > > >> *not* > > >> checking the buffer initially and just check the snprintf return > > value. > > >> That would remove any possibility of us doing an incorrect length > > >> check, as > > >> was the case originally with this patch. > > > That sounds reasonable to me. Please do that. > > I think above check is necessary. Because snprintf returns the total > > length of string > > formated instead of negative when buffer size isn't sufficient. what do > > you think? > > I had the same concern, so I looked it up. > > snprintf() returns the length that the string would have, regardless if it was truncated or not. > > In other words: > > If the string is truncated, snprintf() returns a value greater than the buffer length parameter given to it. > > It can be checked like this: > > if (snprintf(buf, len, "0x%" PRIx64, val) > len) > return -EINVAL; > > In my opinion, checking for negative return values from snprintf() is not necessary. > +1 One nit, because of the \0, we need to check for ">=" len since a return val equal to length means the last character was truncated to make room for the \0. /Bruce