From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f50.google.com (mail-wg0-f50.google.com [74.125.82.50]) by dpdk.org (Postfix) with ESMTP id 4F08D5686 for ; Wed, 14 Jan 2015 16:25:46 +0100 (CET) Received: by mail-wg0-f50.google.com with SMTP id a1so9545219wgh.9 for ; Wed, 14 Jan 2015 07:25:46 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=Hn2qzjDvMvnH7rMbV+M+I9754/K4HFMQ2U5+mp4tc+8=; b=azEan4ryhvWvKp2JQd9nZHhuj3ymCr9A2Iio9Sso0XIgq76LzMlBEw7tnecQvjKlG7 6wykle6dCo/1KvBPsc4avH7Z8eRqUe8+X2tSXJw9NWgQlIvyaQC7wHQRua8hsO19LzQg i60vYcKcJMnDOQmh8QntJuRT818ve1tsaXWsvd7SbOSh1vCDhanZlShRqPdCFJpDVDC6 dma5xCO85tRipoShkjKYKloCaS2aJ1x8IVhO+Yx5/FPTPxFzqKJqHZn2JeD/W3utHzQZ yoq315sJRgW1oX/ago9mG/voOeUyM42vYwqaXd4aKXCdRGyw9L1VZ0b8eFxhM0iXq0Yz TbPQ== X-Gm-Message-State: ALoCoQkzdhbtIHpiqTflKQ2I/1u3rXsAMQeipFksOdHmXy3KFu/oxdzu7m90yxwk/brJ8KQzj474 X-Received: by 10.180.95.4 with SMTP id dg4mr4568264wib.61.1421249144318; Wed, 14 Jan 2015 07:25:44 -0800 (PST) Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136]) by mx.google.com with ESMTPSA id js5sm19074738wid.11.2015.01.14.07.25.40 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 14 Jan 2015 07:25:43 -0800 (PST) From: Thomas Monjalon To: Neil Horman Date: Wed, 14 Jan 2015 16:25:19 +0100 Message-ID: <2338166.O6glFPMYza@xps13> Organization: 6WIND User-Agent: KMail/4.14.3 (Linux/3.17.6-1-ARCH; KDE/4.14.3; x86_64; ; ) In-Reply-To: <1419349913-21674-1-git-send-email-nhorman@tuxdriver.com> References: <1419109299-9603-1-git-send-email-nhorman@tuxdriver.com> <1419349913-21674-1-git-send-email-nhorman@tuxdriver.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v3 1/4] compat: Add infrastructure to support symbol versioning X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Jan 2015 15:25:46 -0000 Hi Neil, 2014-12-23 10:51, Neil Horman: > Add initial pass header files to support symbol versioning. [...] > +# Copyright(c) 2010-2014 Neil Horman Why these dates? > +# All rights reserved. I think this line is not required anymore: http://en.wikipedia.org/wiki/All_rights_reserved [...] > +#ifndef _RTE_COMPAT_H_ > +#define _RTE_COMPAT_H_ Why using underscores? I think it's reserved: http://en.wikipedia.org/wiki/Include_guard#Use_of_.23include_guards > +#define SA(x) #x It should be prefixed. But it's better to use RTE_STR. > +#ifdef RTE_BUILD_SHARED_LIB > + > +/* > + * Provides backwards compatibility when updating exported functions. > + * When a symol is exported from a library to provide an API, it also provides a > + * calling convention (ABI) that is embodied in its name, return type, > + * arguments, etc. On occasion that function may need to change to accomodate > + * new functionality, behavior, etc. When that occurs, it is desireable to > + * allow for backwards compatibility for a time with older binaries that are > + * dynamically linked to the dpdk. to support that the __vsym and Should be "To support that," with uppercase and comma. > + * VERSION_SYMBOL macros are created. They, in conjunction with the > + * _version.map file for a given library allow for multiple versions of > + * a symbol to exist in a shared library so that older binaries need not be > + * immediately recompiled. Their use is outlined in the following example: > + * Assumptions: DPDK 1.(X) contains a function int foo(char *string) > + * DPDK 1.(X+1) needs to change foo to be int foo(int index) > + * > + * To accomplish this: > + * 1) Edit lib//library_version.map to add a DPDK_1.(X+1) node, in which > + * foo is exported as a global symbol. > + * > + * 2) rename the existing function int foo(char *string) to > + * int __vsym foo_v18(char *string) > + * > + * 3) Add this macro immediately below the function > + * VERSION_SYMBOL(foo, _v18, 1.8); > + * > + * 4) Implement a new version of foo. > + * char foo(int value, int otherval) { ...} > + * > + * 5) Mark the newest version as the default version > + * BIND_DEFAULT_SYMBOL(foo, 1.9); > + * > + */ Thanks for this good tutorial. > +#define VERSION_SYMBOL(b, e, v) __asm__(".symver " SA(b) SA(e) ", "SA(b)"@DPDK_"SA(v)) > +#define BASE_SYMBOL(b, n) __asm__(".symver " SA(n) ", "SA(b)"@") > +#define BIND_DEFAULT_SYMBOL(b, v) __asm__(".symver " SA(b) ", "SA(b)"@@DPDK_"SA(v)) > +#define __vsym __attribute__((used)) OK. It would be simpler to read if b, e, v and n were formally defined in a comment. > +#else [...] > +/* > + * RTE_BUILD_SHARED_LIB > + */ This type of comment is strange. It makes me think that we are in the case RTE_BUILD_SHARED_LIB=y > +#endif [...] > + > +CPU_LDFLAGS += --version-script=$(EXPORT_MAP) Why this variable name? VERSION_SCRIPT or VERSION_MAP seems more appropriate. > + > endif > > + Why this newline? > _BUILD = $(LIB) -- Thomas