From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier MATZ Subject: Re: [PATCH 03/15] pmd: Add PMD_REGISTER_DRIVER macro Date: Wed, 16 Apr 2014 18:11:32 +0200 Message-ID: <534EABB4.9020301@6wind.com> References: <1397585169-14537-1-git-send-email-nhorman@tuxdriver.com> <1397585169-14537-4-git-send-email-nhorman@tuxdriver.com> <1462763.GWB5SR3fGh@xps13> <20140416130848.GC11887@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: dev-VfR2kkLFssw@public.gmane.org To: Neil Horman Return-path: In-Reply-To: <20140416130848.GC11887-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" Hi Neil, On 04/16/2014 03:08 PM, Neil Horman wrote: > On Wed, Apr 16, 2014 at 01:52:49PM +0200, Thomas Monjalon wrote: >> 2014-04-15 14:05, Neil Horman: >>> Rather than have each driver have to remember to add a constructor to it to >>> make sure its gets registered properly, wrap that process up in a macro to >>> make registration a one line affair. This also sets the stage for us to >>> make registration of vdev pmds and physical pmds a uniform process >>> >>> Signed-off-by: Neil Horman >> >> Could you explain why having a macro is better than an explicit constructor >> function? >> > Because its a one line declaration inside a driver function that points to the > structure used to initilze the pmd? Having to append ((__constructor__)) to > each initalization function is both error prone during entry and exposes the > possibiilty of developers doing "too much" in their constructor. It also allows > for easy updating to all drivers, if additional boilerplate work needs to be > done in the future for all pmds. Even if it's not critical, in my opinion, the following code is easier to understand: __attribute__((constructor)) static void rte_pmd_ring_init(void) { rte_eal_dev_driver_register(&pmd_ring_drv); } Than: PMD_REGISTER_DRIVER(pmd_ring_drv); The first version explicitly shows what you are doing: defining a static function called at initialization that registers a driver structure. With the second, we're tempted to check what this macro does... My 2 cents, Olivier