From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by dpdk.space (Postfix) with ESMTP id 93BE0A0679
	for <public@inbox.dpdk.org>; Tue,  2 Apr 2019 01:09:42 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 6B135378B;
	Tue,  2 Apr 2019 01:09:42 +0200 (CEST)
Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com
 [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 4F1C53576
 for <dev@dpdk.org>; Tue,  2 Apr 2019 01:09:40 +0200 (CEST)
Received: from compute1.internal (compute1.nyi.internal [10.202.2.41])
 by mailout.nyi.internal (Postfix) with ESMTP id F090A21161;
 Mon,  1 Apr 2019 19:09:39 -0400 (EDT)
Received: from mailfrontend1 ([10.202.2.162])
 by compute1.internal (MEProxy); Mon, 01 Apr 2019 19:09:39 -0400
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h=
 from:to:cc:subject:date:message-id:in-reply-to:references
 :mime-version:content-transfer-encoding:content-type; s=mesmtp;
 bh=F1yzRO9NvS2CHnT0Hlikx/S446sIzo1/QzxoA5rTrhM=; b=bnD+KZkQXmxu
 dbEIm8XOd6Y0+nEgP3zhO/bwS0zXaQj8trq0gd8nLFe5Nl/YQuFWjQFBxIoc5DaY
 /pMB1BJxczxDvMHnJo0R/L9uUfRpciQBwoEhRGgZLD3VToYEuqsxl9npzFrCo+XG
 qipEa2DKSdlBzxnn4bxLLksG33cxNek=
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=
 messagingengine.com; h=cc:content-transfer-encoding:content-type
 :date:from:in-reply-to:message-id:mime-version:references
 :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender
 :x-sasl-enc; s=fm2; bh=F1yzRO9NvS2CHnT0Hlikx/S446sIzo1/QzxoA5rTr
 hM=; b=7w2Y0weNah2iWCoi9av4ltlOypDc7bVJpdUWyQ1Y4ge/Um1iMYrmc4J+Q
 dvcoFBA4tAAF9uDRjGknzB/Ool72enXphrXGr9XQ0PI/GZjymNKfIJb8nonuXfU9
 6wP3TeeDvzgE9QqAnKFnYh8szgfQfc3pwDvQ8Timw4Zfrm5heuq2CL3Cyjuz9ceH
 wYNYp6H6/aCgy73w4tTyA4/jfs+X2exx9kovQG7Z5pA9jXegS97jCsjmx+dKoMzt
 mNRD9Sb/4I7JETXZ/c2sB3Xnmh8Y9YLOWIgHgXoIFAlhh0v8jM25NAMht6Kr5H60
 wwfKn48IqL7PMpGc5+nWxI0ejhqkQ==
X-ME-Sender: <xms:M5qiXPMoE4f9nRq5T2Df4L8x762mOnqpgQ0_teK7syxGF4Mqh1Xctg>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedutddrleehgddujecutefuodetggdotefrodftvf
 curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu
 uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc
 fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs
 ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucffoh
 hmrghinhepughpughkrdhorhhgnecukfhppeejjedrudefgedrvddtfedrudekgeenucfr
 rghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvthenuc
 evlhhushhtvghrufhiiigvpedt
X-ME-Proxy: <xmx:M5qiXMAEy4NwMjV1EoXTrShIIzRo4_6jx_5JMFgF8IYknciCaPYjYg>
 <xmx:M5qiXMaLit2sPe8YUNt-RtpP4Wvh6D4yNfw-VZqcyQFQGa5AfdbcrQ>
 <xmx:M5qiXIrb7gdMSJbvj8nfOYUqkPzHxSPsKsf-lh6TzEW439qG17IwJA>
 <xmx:M5qiXGvsvcgnw1WM9qBrnepEG_kIu5UXD4k_79hNpW1oNhkg1MRT5g>
Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184])
 by mail.messagingengine.com (Postfix) with ESMTPA id 225B5E442F;
 Mon,  1 Apr 2019 19:09:38 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: Anand Rawat <anand.rawat@intel.com>
Cc: dev@dpdk.org, pallavi.kadam@intel.com, ranjit.menon@intel.com,
 jeffrey.b.shaw@intel.com, bruce.richardson@intel.com,
 harini.ramakrishnan@microsoft.com, david.marchand@redhat.com
Date: Tue, 02 Apr 2019 01:09:37 +0200
Message-ID: <1935221.MXycTWvINB@xps>
In-Reply-To: <20190328232451.16988-3-anand.rawat@intel.com>
References: <20190306041634.12976-1-anand.rawat@intel.com>
 <20190328232451.16988-1-anand.rawat@intel.com>
 <20190328232451.16988-3-anand.rawat@intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [dpdk-dev] [PATCH v7 2/8] eal: add header files to support os
	specifics
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
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
Sender: "dev" <dev-bounces@dpdk.org>
Message-ID: <20190401230937.Ky7nF8UR5J7IOcl87H2UyyHI7frah_Q7ZqxCfZWM8Fc@z>

Hi,

29/03/2019 00:24, Anand Rawat:
> Added rte_os.h files to support os specific functionality.
> Updated rte_common.h to include rte_os.h. Updated lib/meson.build to
> inject rte_os.h in every library.
> 
> Signed-off-by: Anand Rawat <anand.rawat@intel.com>
> Signed-off-by: Pallavi Kadam <pallavi.kadam@intel.com>
> Reviewed-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
> Reviewed-by: Ranjit Menon <ranjit.menon@intel.com>
> ---
>  lib/librte_eal/common/include/rte_common.h    |  5 +++-
>  .../common/include/rte_string_fns.h           |  4 ++-
>  .../freebsd/eal/include/exec-env/rte_os.h     | 10 +++++++
>  .../linux/eal/include/exec-env/rte_os.h       |  8 +++++
>  .../windows/eal/include/exec-env/rte_os.h     | 30 +++++++++++++++++++
>  meson.build                                   |  6 ++--

An update of the legacy Makefiles is missing.
It should be something like that:

--- a/lib/librte_eal/freebsd/eal/Makefile
+++ b/lib/librte_eal/freebsd/eal/Makefile
@@ -86,7 +86,7 @@ CFLAGS_eal_thread.o += -Wno-return-type
 CFLAGS_eal_hpet.o += -Wno-return-type
 endif
 
-INC :=  # no bsd specific headers
+INC := rte_os.h
 
 SYMLINK-$(CONFIG_RTE_EXEC_ENV_FREEBSD)-include := $(addprefix include/,$(INC))
 
--- a/lib/librte_eal/linux/eal/Makefile
+++ b/lib/librte_eal/linux/eal/Makefile
@@ -93,7 +93,8 @@ ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
 CFLAGS_eal_thread.o += -Wno-return-type
 endif
 
-INC := rte_kni_common.h
+INC := rte_os.h
+INC += rte_kni_common.h
 
 SYMLINK-$(CONFIG_RTE_EXEC_ENV_LINUX)-include := $(addprefix include/,$(INC))


>  create mode 100644 lib/librte_eal/freebsd/eal/include/exec-env/rte_os.h
>  create mode 100644 lib/librte_eal/linux/eal/include/exec-env/rte_os.h
>  create mode 100644 lib/librte_eal/windows/eal/include/exec-env/rte_os.h
[...]
> +/* os specific include */

Please write OS (uppercase) in this comment and others,
so we can understand it is an acronym.

> +#include <rte_os.h>

You don't prefix with exec-env/ sub-directory, unlike what is done
for rte_kni_common.h, and I think it is a good idea.
However it will require one more patch to make it work with
the make-based system which currently installs the include in exec-env/.
I have just submitted such a patch to remove the exec-env/ directory
both from installed and source hierarchy:
	https://patches.dpdk.org/patch/52031/
Please rebase on top of my patch and move rte_os.h one level upper.
Thanks


One more nit:
> --- /dev/null
> +++ b/lib/librte_eal/windows/eal/include/exec-env/rte_os.h
> +/* macro substitution for windows supported strerror_r */
> +#define strerror_r(a, b, c) strerror_s(b, c, a)
> +
> +/* macro substitution for windows supported strdup */
> +#define strdup(str)	_strdup(str)
> +
> +/* macro substitution for windows supported ssize_t type */
> +typedef SSIZE_T ssize_t;
> +
> +/* macro substitution for windows supported strtok_r */
> +#define strtok_r(str, delim, saveptr) strtok_s(str, delim, saveptr)

Please write Windows with first letter uppercase.
In this file, the comments looks superfluous and can be removed.
Or you can replace by something like "There is no strdup in Microsoft libc."