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

Message ID 20251125230636.157621-1-stephen@networkplumber.org (mailing list archive)
State New
Delegated to: Thomas Monjalon
Headers
Series [RFC,v2] eal: use C library to parse filesystem table |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot-post success github post: success
ci/iol-mellanox-Functional success Functional Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS

Commit Message

Stephen Hemminger Nov. 25, 2025, 11:05 p.m. UTC
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(-)
  

Comments

Konstantin Ananyev Nov. 28, 2025, 1:37 p.m. UTC | #1
> 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
  

Patch

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. */