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 641CBA00C3; Tue, 14 Dec 2021 13:03:12 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E736E40041; Tue, 14 Dec 2021 13:03:11 +0100 (CET) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id E44E34003C for ; Tue, 14 Dec 2021 13:03:09 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1639483390; x=1671019390; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=U2g3JYqR7JnqGUp7ZHGp0KSanMihWa5yizn/fKs2AKg=; b=kdjnxsqW6w/28GIloe8aqDTak5xNqhwtqS1qJ4WVswHSistTX8diKPlg AjwPYecOg63Q9Vlx7/1SUUGEme616XmXWr3VqFcKFluM2VRS4UrR5hqn8 MEjD/U7K1RBZeYuRh9o71pZySGUXZFlslZGL92gsYSpD34UMhBZAFb/KM zLtt1sXOHBQ4gM21RoYPfnf7rfFrknaB4C3jTUDBrpnuEg6Q3JExG7qap dN9UxMjFHEnevuHjq/lVwrfQb9WcVAp2NSiJxQxmO+zzZU5MlM0Unwgh3 VtGCEwYjVAx6fx5s5gmGOmsktugXyumis82AQ71gvtG8YT0Qym+yssHf6 g==; X-IronPort-AV: E=McAfee;i="6200,9189,10197"; a="238774126" X-IronPort-AV: E=Sophos;i="5.88,205,1635231600"; d="scan'208";a="238774126" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Dec 2021 04:03:08 -0800 X-IronPort-AV: E=Sophos;i="5.88,205,1635231600"; d="scan'208";a="567152280" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.19.205]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 14 Dec 2021 04:03:07 -0800 Date: Tue, 14 Dec 2021 12:03:04 +0000 From: Bruce Richardson To: Dmitry Kozlyuk Cc: dev@dpdk.org, aconole@redhat.com Subject: Re: [PATCH] build/eal: add OS defines for C conditional checks Message-ID: References: <20211210145330.108256-1-bruce.richardson@intel.com> <20211214015803.763d30d1@sovereign> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20211214015803.763d30d1@sovereign> 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 01:58:03AM +0300, Dmitry Kozlyuk wrote: > 2021-12-10 14:53 (UTC+0000), Bruce Richardson: > [...] > > Acked-by: Dmitry Kozlyuk > with one typo below and some considerations for the future in the bottom. > > > +Defines to Avoid Conditional Compilation > > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > + > > +In many cases in DPDK, one wants to optionally compile code based on the target platform, > > +or runtime environment. > > "Compile" -> "run", that's the point to use conditionals instead of macros. > > > +While this can be done using the conditional compilation directives, > > +e.g. ``#ifdef RTE_EXEC_ENV_LINUX``, present in DPDK for many releases, > > +this can also be done in many cases using regular ``if`` statements and the following defines: > > > + > > +* ``RTE_ENV_FREEBSD``, ``RTE_ENV_LINUX``, ``RTE_ENV_WINDOWS`` - these define ids for each operating system environment. > > +* ``RTE_EXEC_ENV`` - this defines the id of the current environment, i.e. one of the items in list above. > > +* ``RTE_EXEC_ENV_IS_FREEBSD``, ``RTE_EXEC_ENV_IS_LINUX``, ``RTE_EXEC_ENV_IS_WINDOWS`` - 0/1 values indicating if the current environment is that specified, > > + shortcuts for checking e.g. ``RTE_EXEC_ENV == RTE_ENV_WINDOWS`` > [...] > > I wonder whether #if RTE_EXEC_ENV_IS_xxx > should be preferred over #ifdef RTE_EXEC_ENV_xxx, > so that all checks use the same symbol > (and then remove old macros one day). > > Since C conditionals are preferred over #ifdef, > I suggest to give pointers when to use one or another mechanism: > > If a code fragment can compile on all platforms, > but cannot run on some due to lack of support, > branch on constants. > > If a code fragment cannot compile on all platforms > (e.g. use of OS-specific headers or macros), > but constitutes only a small fraction of the file, > use conditional compilation. > > If a group of functions implement an interface > in an OS- or platform-specific way, > create a file for each of the supported environments > and plug an appropriate file from ``meson.build``. Good suggestions. I rework into a v2 when I get the chance. /Bruce