Hi Jan, > -----Original Message----- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Jan Viktorin > Sent: Friday, May 06, 2016 7:18 PM > To: dev at dpdk.org > Cc: Jan Viktorin <viktorin at rehivetech.com>; David Marchand > <david.marchand at 6wind.com>; Thomas Monjalon <thomas.monjalon at 6wind.com>; > Bruce Richardson <bruce.richardson at intel.com>; Declan Doherty > <declan.doherty at intel.com>; jianbo.liu at linaro.org; > jerin.jacob at caviumnetworks.com; Keith Wiles <keith.wiles at intel.com>; > Stephen > Hemminger <stephen at networkplumber.org> > 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 <viktorin at rehivetech.com> > --- > 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 <stdio.h> > #include <stdint.h> > #include <limits.h> > #include <unistd.h> > @@ -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)
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 almost skipped the '..f' in the name and wondered how two functions having same name exist :D > +{ > + char buf[BUFSIZ]; > + char *end = NULL; > + > + RTE_VERIFY(f != NULL); > + > + if (fgets(buf, sizeof(buf), f) == NULL) > + return -1; > + > + *val = strtoul(buf, &end, 0); > + if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) > + return -2; > + > + return 0; > +} > + > /* parse a sysfs (or other) file containing one integer value */ > int > eal_parse_sysfs_value(const char *filename, unsigned long *val) > { > + int ret; > FILE *f; > - char buf[BUFSIZ]; > - char *end = NULL; > > if ((f = fopen(filename, "r")) == NULL) { > RTE_LOG(ERR, EAL, "%s(): cannot open sysfs value %s\n", > @@ -140,21 +157,18 @@ eal_parse_sysfs_value(const char *filename, unsigned > long *val) > return -1; > } > > - if (fgets(buf, sizeof(buf), f) == NULL) { > + ret = eal_parse_sysfs_valuef(f, val); > + if (ret == -1) { > RTE_LOG(ERR, EAL, "%s(): cannot read sysfs value %s\n", > - __func__, filename); > - fclose(f); > - return -1; > + __func__, filename); > } > - *val = strtoul(buf, &end, 0); > - if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) { > + else if (ret < 0) { > RTE_LOG(ERR, EAL, "%s(): cannot parse sysfs value %s\n", > __func__, filename); > - fclose(f); > - return -1; > } > + > fclose(f); > - return 0; > + return ret; > } > > > -- > 2.8.0 - Shreyansh