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 94CF4A00C3; Wed, 15 Dec 2021 10:35:51 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 255BF40688; Wed, 15 Dec 2021 10:35:51 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id D793340041 for ; Wed, 15 Dec 2021 10:35:49 +0100 (CET) X-MimeOLE: Produced By Microsoft Exchange V6.5 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 01/12] net: add string to IPv4 parse function Date: Wed, 15 Dec 2021 10:35:44 +0100 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D86D77@smartserver.smartshare.dk> In-Reply-To: X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH 01/12] net: add string to IPv4 parse function Thread-Index: Adfxlghzrfc8248YT5Cuy8H0QlD1rwAAEeZQ References: <20211214141242.3383831-1-ronan.randles@intel.com> <20211214141242.3383831-2-ronan.randles@intel.com> <98CBD80474FA8B44BF855DF32C47DC35D86D6E@smartserver.smartshare.dk> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Bruce Richardson" Cc: "Ronan Randles" , , 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: Bruce Richardson [mailto:bruce.richardson@intel.com] > Sent: Wednesday, 15 December 2021 10.27 >=20 > On Tue, Dec 14, 2021 at 06:31:06PM +0100, Morten Br=F8rup 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. That would require additional lines of code in the application. I would = rather have that additional code inside the utility function. There is = no need to keep the function simple.