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 3557EA00C3; Wed, 15 Dec 2021 10:27:47 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AF62040688; Wed, 15 Dec 2021 10:27:46 +0100 (CET) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mails.dpdk.org (Postfix) with ESMTP id 9FEF740041 for ; Wed, 15 Dec 2021 10:27:45 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1639560465; x=1671096465; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=BeQYjqo0dWWKbrZWdEc6sd7Ob/2wiUO4LHpMDIKgVs0=; b=IWsTEDvcRGI1H8RhcxaeINyvUIaHWVeUkyv4UvqeBpjPlLf6v03pbn1r ILR5UWpybAJ/Evypgpy/wc4CMjFT9GdeTfTJ8aopjvycq0KPnZZKNa1wo joi3NutrJVdytouTvShSQgpY6GPUKpIMRpoXhZW38IVqrg3dOReHsMKpn Z3c50X7ZCSmtLZpUajkzYJ1//HHfVkGN5CxXw6Q5yFjN3AmdT+V6DOybn VsO8Ltuipz7qAZkivcW1Vu7GjmvwbYDQTjPauQM31tAk42eNz6kqIwzaN dtB++gL+LX9FpcXZDx4cDZTYBTEJmlCbV7MQ4niwfEEbwN11KEmlNgAR2 A==; X-IronPort-AV: E=McAfee;i="6200,9189,10198"; a="226468930" X-IronPort-AV: E=Sophos;i="5.88,207,1635231600"; d="scan'208";a="226468930" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Dec 2021 01:27:34 -0800 X-IronPort-AV: E=Sophos;i="5.88,207,1635231600"; d="scan'208";a="465496734" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.7.115]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 15 Dec 2021 01:27:33 -0800 Date: Wed, 15 Dec 2021 09:27:29 +0000 From: Bruce Richardson To: Morten =?iso-8859-1?Q?Br=F8rup?= Cc: Ronan Randles , dev@dpdk.org, harry.van.haaren@intel.com Subject: Re: [PATCH 01/12] net: add string to IPv4 parse function Message-ID: References: <20211214141242.3383831-1-ronan.randles@intel.com> <20211214141242.3383831-2-ronan.randles@intel.com> <98CBD80474FA8B44BF855DF32C47DC35D86D6E@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D86D6E@smartserver.smartshare.dk> 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 Tue, Dec 14, 2021 at 06:31:06PM +0100, Morten Brørup wrote: > > From: Ronan Randles [mailto:ronan.randles@intel.com] > > Sent: Tuesday, 14 December 2021 15.13 > > > > Added function that accepts ip string as a parameter and returns an ip > > address represented by a uint32_t. Relevant unit test for this function > > is also included. > > > > Signed-off-by: Harry van Haaren > > Signed-off-by: Ronan Randles > > --- > > [snip] > > > diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h > > index c575250852..188054fda4 100644 > > --- a/lib/net/rte_ip.h > > +++ b/lib/net/rte_ip.h > > @@ -426,6 +426,24 @@ rte_ipv4_udptcp_cksum_verify(const struct > > rte_ipv4_hdr *ipv4_hdr, > > return 0; > > } > > > > +/** > > + * IP address parser. > > + * > > + * @param src_ip > > + * The IP address to be parsed. > > + * @param output_addr > > + * The array in which the parsed digits will be saved. > > + * > > + * @retval 0 > > + * Success. > > + * @retval -1 > > + * Failure due to invalid input arguments. > > + */ > > + > > +__rte_experimental > > +int32_t > > +rte_ip_parse_addr(const char *src_ip, uint32_t *output_addr); > > + > > Good initiative! > > This should set a precedent for to/from string functions, so be careful about names and calling conventions. > > I have a few suggestions: > > The function should take a parameter to tell if the input string must be zero-terminated or not. This is highly useful for parsing subnet strings (e.g. "192.0.2.0/24") and IP range strings (e.g. "192.0.2.2-192.0.2.253"). > > The return value should be the number of characters read from the input string, and still -1 on error. With this modification, also consider using the return type ssize_t instead of int32_t. > Interesting point on the "must be zero-terminated" parameter. However, if the return value is the number of characters read, then the user can check for null-termination very easily after the call, and not need the parameter. Therefore, I would suggest having the function always assume chars after the address and let the user check for null if needed afterwards, to keep function simpler. /Bruce