From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from wes1-so1.wedos.net (wes1-so1.wedos.net [46.28.106.15]) by dpdk.org (Postfix) with ESMTP id 2AA072BA0 for ; Fri, 6 May 2016 18:22:35 +0200 (CEST) Received: from pcviktorin.fit.vutbr.cz (pcviktorin.fit.vutbr.cz [147.229.13.147]) by wes1-so1.wedos.net (Postfix) with ESMTPSA id 3r1cWG57ctz5TR; Fri, 6 May 2016 18:22:34 +0200 (CEST) Date: Fri, 6 May 2016 18:20:42 +0200 From: Jan Viktorin To: Thomas Monjalon Cc: dev@dpdk.org, Bruce Richardson , David Marchand Message-ID: <20160506182042.411cf755@pcviktorin.fit.vutbr.cz> In-Reply-To: <7154696.NNSjZHJaDi@xps13> References: <1462531720-26217-1-git-send-email-viktorin@rehivetech.com> <1462531720-26217-2-git-send-email-viktorin@rehivetech.com> <7154696.NNSjZHJaDi@xps13> Organization: RehiveTech MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v1 01/10] app/test: introduce resources for tests 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: Fri, 06 May 2016 16:22:35 -0000 On Fri, 06 May 2016 16:01:08 +0200 Thomas Monjalon wrote: > 2016-05-06 12:48, Jan Viktorin: > > --- /dev/null > > +++ b/app/test/resource.h > > @@ -0,0 +1,61 @@ > > +/*- > > + * BSD LICENSE > [...] > > + */ > > Please include a multi-line comment here to explain what is a resource > and why it is needed. Will fix. > > > + > > +#ifndef _RESOURCE_H_ > > +#define _RESOURCE_H_ > > + > |...] > |> + > > +TAILQ_HEAD(resource_list, resource); > > +extern struct resource_list resource_list; > > Why extern? Will fix. > > > + > > +struct resource { > > + const char *name; > > + const char *beg; > > begin? > Bruce is removing some megabytes from lpm tests, so we have > some room for field names ;) I love 3-chars identifiers... ;) I will rework this, thanks. > > > + const char *end; > > + TAILQ_ENTRY(resource) next; > > +}; > > + > > +static inline size_t resource_size(const struct resource *r) > > Why inline? It could be in .c OK. > > > +{ > > + return r->end - r->beg; > > +} > > + > > +const struct resource *resource_find(const char *name); > > + > > +void __resource_register(struct resource *r); > > A comment is needed to explain the role of this function. > Why a double underscore? A non-API function. It shouldn't be called by user. Nothing more. > > > +#define REGISTER_RESOURCE(_n, _b, _e) \ > > I'm not a big fan of the underscores. +1 > > > +static struct resource linkres_ ##_n = { \ > > + .name = RTE_STR(_n), \ > > + .beg = _b, \ > > + .end = _e, \ > > +}; \ > > +__REGISTER_RESOURCE(linkres_ ##_n) > > Please avoid nested macros. I don't understand this. I am avoiding code duplication. Is it wrong? > > > +#define __REGISTER_RESOURCE(name) \ > > +static void __attribute__((constructor,used)) resinitfn_ ##name(void); \ > > Why declaring the function just before its implementation? Wrote in a hurry to work out-of-the box. Will fix... > > > +static void __attribute__((constructor,used)) resinitfn_ ##name(void) \ > > +{ \ > > + __resource_register(&name); \ > > +} > -- Jan Viktorin E-mail: Viktorin@RehiveTech.com System Architect Web: www.RehiveTech.com RehiveTech Brno, Czech Republic