From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from wes1-so2.wedos.net (wes1-so2.wedos.net [46.28.106.16]) by dpdk.org (Postfix) with ESMTP id 892CC37A6 for ; Mon, 13 Jun 2016 16:29:39 +0200 (CEST) Received: from pcviktorin.fit.vutbr.cz (pcviktorin.fit.vutbr.cz [147.229.13.147]) by wes1-so2.wedos.net (Postfix) with ESMTPSA id 3rSwCR0cDNzC4b; Mon, 13 Jun 2016 16:29:38 +0200 (CEST) Date: Mon, 13 Jun 2016 16:24:31 +0200 From: Jan Viktorin To: Shreyansh Jain Cc: "dev@dpdk.org" , David Marchand , Thomas Monjalon , Bruce Richardson , Declan Doherty , "jianbo.liu@linaro.org" , "jerin.jacob@caviumnetworks.com" , "Keith Wiles" , Stephen Hemminger Message-ID: <20160613162431.714e56ae@pcviktorin.fit.vutbr.cz> In-Reply-To: References: Organization: RehiveTech MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v1 02/28] eal: extract function eal_parse_sysfs_valuef 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: Mon, 13 Jun 2016 14:29:39 -0000 On Mon, 13 Jun 2016 14:18:40 +0000 Shreyansh Jain wrote: > Hi Jan, > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jan Viktorin > > Sent: Friday, May 06, 2016 7:18 PM > > To: dev@dpdk.org > > Cc: Jan Viktorin ; David Marchand > > ; Thomas Monjalon ; > > Bruce Richardson ; Declan Doherty > > ; jianbo.liu@linaro.org; > > jerin.jacob@caviumnetworks.com; Keith Wiles ; Stephen > > Hemminger > > Subject: [dpdk-dev] [PATCH v1 02/28] eal: extract function > > eal_parse_sysfs_valuef > > > > The eal_parse_sysfs_value function accepts a filename however, such interface > > introduces race-conditions to the code. Introduce the variant of this > > function > > that accepts an already opened file instead of a filename. > > > > Signed-off-by: Jan Viktorin > > --- > > lib/librte_eal/common/eal_filesystem.h | 5 +++++ > > lib/librte_eal/linuxapp/eal/eal.c | 36 +++++++++++++++++++++++--------- > > -- > > 2 files changed, 30 insertions(+), 11 deletions(-) > > > > diff --git a/lib/librte_eal/common/eal_filesystem.h > > b/lib/librte_eal/common/eal_filesystem.h > > index fdb4a70..7875454 100644 > > --- a/lib/librte_eal/common/eal_filesystem.h > > +++ b/lib/librte_eal/common/eal_filesystem.h > > @@ -43,6 +43,7 @@ > > /** Path of rte config file. */ > > #define RUNTIME_CONFIG_FMT "%s/.%s_config" > > > > +#include > > #include > > #include > > #include > > @@ -115,4 +116,8 @@ eal_get_hugefile_temp_path(char *buffer, size_t buflen, > > const char *hugedir, int > > * Used to read information from files on /sys */ > > int eal_parse_sysfs_value(const char *filename, unsigned long *val); > > > > +/** Function to read a single numeric value from a file on the filesystem. > > + * Used to read information from files on /sys */ > > +int eal_parse_sysfs_valuef(FILE *f, unsigned long *val); > > + > > #endif /* EAL_FILESYSTEM_H */ > > diff --git a/lib/librte_eal/linuxapp/eal/eal.c > > b/lib/librte_eal/linuxapp/eal/eal.c > > index 4b28197..e8fce6b 100644 > > --- a/lib/librte_eal/linuxapp/eal/eal.c > > +++ b/lib/librte_eal/linuxapp/eal/eal.c > > @@ -126,13 +126,30 @@ rte_eal_get_configuration(void) > > return &rte_config; > > } > > > > +int > > +eal_parse_sysfs_valuef(FILE *f, unsigned long *val) Hi Shreyansh, > > Trivial Comment: Maybe it is just me, but this function name is too close to its caller 'eal_parse_sysfs_value'. Probably, the name of the caller can be changed to 'eal_parse_sysfs' because anyways value parsing is being done in this ('eal_parse_sysfs_valuef()) function now. And, of course, dropping the '..f' in this name. I don't like the idea of renaming the orignal function eal_parse_sysfs_value. The function name is not related to its actual body but to its semantics. And the semantics are still the same. This would introduce many other unneeded changes... > > I almost skipped the '..f' in the name and wondered how two functions having same name exist :D I agree that a better name would be nice here. This convention was based on the libc naming (fopen, fclose) but the "f" letter could not be at the beginning. What about one of those? * eal_parse_sysfs_fd_value * eal_parse_sysfs_file_value Regards Jan [...]