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 B3918A034C;
	Tue, 14 Dec 2021 18:31:11 +0100 (CET)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 5287140041;
	Tue, 14 Dec 2021 18:31:11 +0100 (CET)
Received: from smartserver.smartsharesystems.com
 (smartserver.smartsharesystems.com [77.243.40.215])
 by mails.dpdk.org (Postfix) with ESMTP id 261FA4003C
 for <dev@dpdk.org>; Tue, 14 Dec 2021 18:31:10 +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: Tue, 14 Dec 2021 18:31:06 +0100
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D86D6E@smartserver.smartshare.dk>
In-Reply-To: <20211214141242.3383831-2-ronan.randles@intel.com>
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
Thread-Topic: [PATCH 01/12] net: add string to IPv4 parse function
Thread-Index: Adfw9LU4qn0UCC2YQoi61dhEeCCD7gAF4o3Q
References: <20211214141242.3383831-1-ronan.randles@intel.com>
 <20211214141242.3383831-2-ronan.randles@intel.com>
From: =?iso-8859-1?Q?Morten_Br=F8rup?= <mb@smartsharesystems.com>
To: "Ronan Randles" <ronan.randles@intel.com>,
	<dev@dpdk.org>
Cc: <harry.van.haaren@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

> From: Ronan Randles [mailto:ronan.randles@intel.com]
> Sent: Tuesday, 14 December 2021 15.13
>=20
> 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.
>=20
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> Signed-off-by: Ronan Randles <ronan.randles@intel.com>
> ---

[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;
>  }
>=20
> +/**
> + * 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.

-Morten