[dpdk-dev] [PATCH v1 02/28] eal: extract function eal_parse_sysfs_valuef
Shreyansh Jain
shreyansh.jain at nxp.com
Mon Jun 13 16:18:40 CEST 2016
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
More information about the dev
mailing list