From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 4F1C53576 for ; 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: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedutddrleehgddujecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucffoh hmrghinhepughpughkrdhorhhgnecukfhppeejjedrudefgedrvddtfedrudekgeenucfr rghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvthenuc evlhhushhtvghrufhiiigvpedt X-ME-Proxy: 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 To: Anand Rawat 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="us-ascii" 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Apr 2019 23:09:40 -0000 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 > Signed-off-by: Pallavi Kadam > Reviewed-by: Jeff Shaw > Reviewed-by: Ranjit Menon > --- > 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 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." From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 93BE0A0679 for ; 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 ; 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: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedutddrleehgddujecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucffoh hmrghinhepughpughkrdhorhhgnecukfhppeejjedrudefgedrvddtfedrudekgeenucfr rghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvthenuc evlhhushhtvghrufhiiigvpedt X-ME-Proxy: 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 To: Anand Rawat 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > Signed-off-by: Pallavi Kadam > Reviewed-by: Jeff Shaw > Reviewed-by: Ranjit Menon > --- > 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 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."