Re: [PATCH i-g-t 7/8] lib/igt_drm_fdinfo: Remove prefix arg from parse functions

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Tue, Apr 02, 2024 at 03:17:15PM -0700, Lucas De Marchi wrote:
>The prefix is not really needed and __igt_parse_drm_fdinfo()
>can simply pass l + keylen as argument which simplifies the parse
>functions.
>
>While refactoring this, it also replaces the remaining calls to index()
>to use strchr().
>
>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>---
> lib/igt_drm_fdinfo.c | 69 ++++++++++++++++++--------------------------
> 1 file changed, 28 insertions(+), 41 deletions(-)
>
>diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
>index a0b41e956..7a9ae94f0 100644
>--- a/lib/igt_drm_fdinfo.c
>+++ b/lib/igt_drm_fdinfo.c
>@@ -61,25 +61,23 @@ static const char *ignore_space(const char *s)
> 	return s;
> }
>
>-static int parse_engine(char *line, struct drm_client_fdinfo *info,
>-			size_t prefix_len,
>+static int parse_engine(const char *name, struct drm_client_fdinfo *info,
> 			const char **name_map, unsigned int map_entries,
> 			uint64_t *val)
> {
>-	ssize_t name_len;
>-	char *name, *p;
>+	const char *p;
>+	size_t name_len;
> 	int found = -1;
> 	unsigned int i;
>
>-	p = index(line, ':');
>-	if (!p || p == line)
>+	p = strchr(name, ':');
>+	if (!p)
> 		return -1;
>
>-	name_len = p - line - prefix_len;
>+	name_len = p - name;
> 	if (name_len < 1)
> 		return -1;
>-
>-	name = line + prefix_len;
>+	p++;
>
> 	if (name_map) {
> 		for (i = 0; i < map_entries; i++) {
>@@ -105,40 +103,30 @@ static int parse_engine(char *line, struct drm_client_fdinfo *info,
> 	}
>
> 	if (found >= 0) {
>-		while (*++p && isspace(*p));
>+		p = ignore_space(p);
> 		*val = strtoull(p, NULL, 10);
> 	}
>
> 	return found;
> }
>
>-static const char *ignore_space(const char *s)
>-{
>-	for (; *s && isspace(*s); s++)
>-		;
>-
>-	return s;
>-}

Removal of this ignore_space should be in patch 6 (like Kamil 
mentioned).

The rest of the logic is same as the previous code, so with the above 
change this is

Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

>-
>-static int parse_region(char *line, struct drm_client_fdinfo *info,
>-			size_t prefix_len,
>+static int parse_region(const char *name, struct drm_client_fdinfo *info,
> 			const char **region_map, unsigned int region_entries,
> 			uint64_t *val)
> {
>-	char *name, *p, *unit = NULL;
>-	ssize_t name_len;
>+	const char *p;
>+	size_t name_len;
> 	int found = -1;
> 	unsigned int i;
>
>-	p = index(line, ':');
>-	if (!p || p == line)
>+	p = strchr(name, ':');
>+	if (!p)
> 		return -1;
>
>-	name_len = p - line - prefix_len;
>+	name_len = p - name;
> 	if (name_len < 1)
> 		return -1;
>-
>-	name = line + prefix_len;
>+	p++;
>
> 	if (region_map) {
> 		for (i = 0; i < region_entries; i++) {
>@@ -170,20 +158,19 @@ static int parse_region(char *line, struct drm_client_fdinfo *info,
> 	if (found < 0)
> 		goto out;
>
>-	while (*++p && isspace(*p))
>-		;
>+	p = ignore_space(p);
> 	*val = strtoull(p, NULL, 10);
>
>-	p = index(p, ' ');
>+	p = strchr(p, ' ');
> 	if (!p)
> 		goto out;
>+	p++;
>
>-	unit = ++p;
>-	if (!strcmp(unit, "KiB")) {
>+	if (!strcmp(p, "KiB")) {
> 		*val *= 1024;
>-	} else if (!strcmp(unit, "MiB")) {
>+	} else if (!strcmp(p, "MiB")) {
> 		*val *= 1024 * 1024;
>-	} else if (!strcmp(unit, "GiB")) {
>+	} else if (!strcmp(p, "GiB")) {
> 		*val *= 1024 * 1024 * 1024;
> 	}
>
>@@ -251,14 +238,14 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
> 			v = ignore_space(v);
> 			strncpy(info->pdev, v, sizeof(info->pdev) - 1);
> 		} else if (strstartswith(l, "drm-engine-capacity-", &keylen)) {
>-			idx = parse_engine(l, info, keylen,
>+			idx = parse_engine(l + keylen, info,
> 					   name_map, map_entries, &val);
> 			if (idx >= 0) {
> 				info->capacity[idx] = val;
> 				num_capacity++;
> 			}
> 		} else if (strstartswith(l, "drm-engine-", &keylen)) {
>-			idx = parse_engine(l, info, keylen,
>+			idx = parse_engine(l + keylen, info,
> 					   name_map, map_entries, &val);
> 			if (idx >= 0) {
> 				if (!info->capacity[idx])
>@@ -269,23 +256,23 @@ __igt_parse_drm_fdinfo(int dir, const char *fd, struct drm_client_fdinfo *info,
> 					info->last_engine_index = idx;
> 			}
> 		} else if (strstartswith(l, "drm-total-", &keylen)) {
>-			idx = parse_region(l, info, keylen,
>+			idx = parse_region(l + keylen, info,
> 					   region_map, region_entries, &val);
> 			UPDATE_REGION(idx, total, val);
> 		} else if (strstartswith(l, "drm-shared-", &keylen)) {
>-			idx = parse_region(l, info, keylen,
>+			idx = parse_region(l + keylen, info,
> 					   region_map, region_entries, &val);
> 			UPDATE_REGION(idx, shared, val);
> 		} else if (strstartswith(l, "drm-resident-", &keylen)) {
>-			idx = parse_region(l, info, keylen,
>+			idx = parse_region(l + keylen, info,
> 					   region_map, region_entries, &val);
> 			UPDATE_REGION(idx, resident, val);
> 		} else if (strstartswith(l, "drm-purgeable-", &keylen)) {
>-			idx = parse_region(l, info, keylen,
>+			idx = parse_region(l + keylen, info,
> 					   region_map, region_entries, &val);
> 			UPDATE_REGION(idx, purgeable, val);
> 		} else if (strstartswith(l, "drm-active-", &keylen)) {
>-			idx = parse_region(l, info, keylen,
>+			idx = parse_region(l + keylen, info,
> 					   region_map, region_entries, &val);
> 			UPDATE_REGION(idx, active, val);
> 		}
>-- 
>2.43.0
>