RE: [RFC v2] eal: use C library to parse filesystem table

[Konstantin Ananyev <konstantin.ananyev@huawei.com>]

> Rather than doing parsing of /proc/mounts with open coded string
> handling, use the standard C library routines.
> These exist in BSD and Linux.
> 
> It also avoids any possible issues with escaped strings etc
> which the library handles. See getmntent(3) man page.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> v2 - implement suggestion to simplify conditionals
> 
>  lib/eal/linux/eal_hugepage_info.c | 58 ++++++++++---------------------
>  1 file changed, 18 insertions(+), 40 deletions(-)
> 
> diff --git a/lib/eal/linux/eal_hugepage_info.c
> b/lib/eal/linux/eal_hugepage_info.c
> index d47a19c56a..7161b1a2fb 100644
> --- a/lib/eal/linux/eal_hugepage_info.c
> +++ b/lib/eal/linux/eal_hugepage_info.c
> @@ -13,6 +13,7 @@
>  #include <inttypes.h>
>  #include <unistd.h>
>  #include <errno.h>
> +#include <mntent.h>
>  #include <sys/mman.h>
>  #include <sys/stat.h>
> 
> @@ -195,23 +196,13 @@ get_default_hp_size(void)
>  static int
>  get_hugepage_dir(uint64_t hugepage_sz, char *hugedir, int len)
>  {
> -	enum proc_mount_fieldnames {
> -		DEVICE = 0,
> -		MOUNTPT,
> -		FSTYPE,
> -		OPTIONS,
> -		_FIELDNAME_MAX
> -	};
>  	static uint64_t default_size = 0;
>  	const char proc_mounts[] = "/proc/mounts";
> -	const char hugetlbfs_str[] = "hugetlbfs";
> -	const size_t htlbfs_str_len = sizeof(hugetlbfs_str) - 1;
>  	const char pagesize_opt[] = "pagesize=";
>  	const size_t pagesize_opt_len = sizeof(pagesize_opt) - 1;
> -	const char split_tok = ' ';
> -	char *splitstr[_FIELDNAME_MAX];
>  	char found[PATH_MAX] = "";
>  	char buf[BUFSIZ];
> +	struct mntent entry;
>  	const struct internal_config *internal_conf =
>  		eal_get_internal_configuration();
>  	const size_t hugepage_dir_len = (internal_conf->hugepage_dir != NULL)
> ?
> @@ -226,56 +217,43 @@ get_hugepage_dir(uint64_t hugepage_sz, char
> *hugedir, int len)
>  		return -1;
>  	}
> 
> -	FILE *fd = fopen(proc_mounts, "r");
> -	if (fd == NULL)
> +	FILE *mounts = setmntent(proc_mounts, "r");
> +	if (mounts == NULL)
>  		rte_panic("Cannot open %s\n", proc_mounts);
> 
>  	if (default_size == 0)
>  		default_size = get_default_hp_size();
> 
> -	while (fgets(buf, sizeof(buf), fd)){
> +	while (getmntent_r(mounts, &entry, buf, sizeof(buf))) {
>  		const char *pagesz_str;
> -		size_t mountpt_len = 0;
> -
> -		if (rte_strsplit(buf, sizeof(buf), splitstr, _FIELDNAME_MAX,
> -				split_tok) != _FIELDNAME_MAX) {
> -			EAL_LOG(ERR, "Error parsing %s", proc_mounts);
> -			break; /* return NULL */
> -		}
> +		uint64_t pagesz = default_size;
> +		size_t mountpt_len;
> 
> -		if (strncmp(splitstr[FSTYPE], hugetlbfs_str, htlbfs_str_len) != 0)
> +		if (strcmp(entry.mnt_type, "hugetlbfs") != 0)
>  			continue;
> 
> -		pagesz_str = strstr(splitstr[OPTIONS], pagesize_opt);
> +		pagesz_str = strstr(entry.mnt_opts, pagesize_opt);
> +		if (pagesz_str)
> +			pagesz =
> rte_str_to_size(&pagesz_str[pagesize_opt_len]);
> 
> -		/* if no explicit page size, the default page size is compared */
> -		if (pagesz_str == NULL) {
> -			if (hugepage_sz != default_size)
> -				continue;
> -		}
> -		/* there is an explicit page size, so check it */
> -		else {
> -			uint64_t pagesz =
> rte_str_to_size(&pagesz_str[pagesize_opt_len]);
> -			if (pagesz != hugepage_sz)
> -				continue;
> -		}
> +		if (pagesz != hugepage_sz)
> +			continue;
> 
>  		/*
>  		 * If no --huge-dir option has been given, we're done.
>  		 */
>  		if (internal_conf->hugepage_dir == NULL) {
> -			strlcpy(found, splitstr[MOUNTPT], len);
> +			strlcpy(found, entry.mnt_dir, len);
>  			break;
>  		}
> 
> -		mountpt_len = strlen(splitstr[MOUNTPT]);
> +		mountpt_len = strlen(entry.mnt_dir);
> 
>  		/*
>  		 * Ignore any mount that doesn't contain the --huge-dir directory
>  		 * or where mount point is not a parent path of --huge-dir
>  		 */
> -		if (strncmp(internal_conf->hugepage_dir, splitstr[MOUNTPT],
> -				mountpt_len) != 0 ||
> +		if (strncmp(internal_conf->hugepage_dir, entry.mnt_dir,
> mountpt_len) != 0 ||
>  			(hugepage_dir_len > mountpt_len &&
>  				internal_conf->hugepage_dir[mountpt_len] !=
> '/')) {
>  			continue;
> @@ -286,10 +264,10 @@ get_hugepage_dir(uint64_t hugepage_sz, char
> *hugedir, int len)
>  		 * (so /mnt/1 is preferred over /mnt for matching /mnt/1/2)).
>  		 */
>  		if (mountpt_len > strlen(found))
> -			strlcpy(found, splitstr[MOUNTPT], len);
> +			strlcpy(found, entry.mnt_dir, len);
>  	} /* end while fgets */
> 
> -	fclose(fd);
> +	endmntent(mounts);
> 
>  	if (found[0] != '\0') {
>  		/* If needed, return the requested dir, not the mount point. */
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>

> 2.51.0